From 56663b38e67d891cbee23c590458610f98837e69 Mon Sep 17 00:00:00 2001 From: Nisha Agarwal Date: Tue, 22 Sep 2015 03:03:51 -0700 Subject: [PATCH] Add CLI support for RAID configuration This commits add the following: * Ability to set target_raid_config for a node using node-set-target-raid-config. * Display target_raid_config and raid_config in node-show. * Display logical disk properties for a driver using driver-raid-logical-disk-properties. NOTE: Client still defaults to version 1.9, so '--ironic-api-version 1.15' (or above) should be added to the CLI to use the feature as RAID can be triggered only with manual cleaning which uses the ironic-api-version 1.15. Co-Authored-By: Nisha Agarwal Closes_bug: 1526400 Change-Id: Id7bb8a242838029e4cc3715d18b27fb1d5cd4dfa --- .../tests/unit/osc/v1/test_baremetal.py | 6 +- ironicclient/tests/unit/v1/test_driver.py | 33 +++++++- .../tests/unit/v1/test_driver_shell.py | 24 ++++++ ironicclient/tests/unit/v1/test_node.py | 21 +++++ ironicclient/tests/unit/v1/test_node_shell.py | 84 ++++++++++++++++--- ironicclient/v1/driver.py | 19 +++++ ironicclient/v1/driver_shell.py | 16 ++++ ironicclient/v1/node.py | 11 +++ ironicclient/v1/node_shell.py | 54 ++++++++---- ironicclient/v1/resource_fields.py | 6 ++ .../raid_CLI_support-7e816ccd0fb31d2b.yaml | 17 ++++ 11 files changed, 264 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/raid_CLI_support-7e816ccd0fb31d2b.yaml diff --git a/ironicclient/tests/unit/osc/v1/test_baremetal.py b/ironicclient/tests/unit/osc/v1/test_baremetal.py index b97ae43..f116e43 100644 --- a/ironicclient/tests/unit/osc/v1/test_baremetal.py +++ b/ironicclient/tests/unit/osc/v1/test_baremetal.py @@ -275,8 +275,10 @@ class TestBaremetalList(TestBaremetal): 'Driver Internal Info', 'Extra', 'Instance Info', 'Instance UUID', 'Last Error', 'Maintenance', 'Maintenance Reason', 'Power State', 'Properties', - 'Provisioning State', 'Provision Updated At', 'Reservation', + 'Provisioning State', 'Provision Updated At', + 'Current RAID configuration', 'Reservation', 'Target Power State', 'Target Provision State', + 'Target RAID configuration', 'Updated At', 'Inspection Finished At', 'Inspection Started At', 'UUID', 'Name') self.assertEqual(collist, columns) @@ -304,6 +306,8 @@ class TestBaremetalList(TestBaremetal): '', '', '', + '', + '', baremetal_fakes.baremetal_uuid, baremetal_fakes.baremetal_name, ), ) diff --git a/ironicclient/tests/unit/v1/test_driver.py b/ironicclient/tests/unit/v1/test_driver.py index 22d6809..995f49d 100644 --- a/ironicclient/tests/unit/v1/test_driver.py +++ b/ironicclient/tests/unit/v1/test_driver.py @@ -35,6 +35,10 @@ DRIVER_VENDOR_PASSTHRU_METHOD = {"lookup": {"attach": "false", "http_methods": ["POST"], "description": "", "async": "false"}} +DRIVER2_RAID_LOGICAL_DISK_PROPERTIES = { + "property1": "description1", + "property2": "description2", +} fake_responses = { '/v1/drivers': @@ -64,7 +68,14 @@ fake_responses = { {}, DRIVER_VENDOR_PASSTHRU_METHOD, ), - } + }, + '/v1/drivers/%s/raid/logical_disk_properties' % DRIVER2['name']: + { + 'GET': ( + {}, + DRIVER2_RAID_LOGICAL_DISK_PROPERTIES, + ), + }, } @@ -100,6 +111,26 @@ class DriverManagerTest(testtools.TestCase): self.assertEqual(expect, self.api.calls) self.assertEqual(DRIVER2_PROPERTIES, properties) + def test_driver_raid_logical_disk_properties(self): + properties = self.mgr.raid_logical_disk_properties(DRIVER2['name']) + expect = [ + ('GET', + '/v1/drivers/%s/raid/logical_disk_properties' % DRIVER2['name'], + {}, None)] + self.assertEqual(expect, self.api.calls) + self.assertEqual(DRIVER2_RAID_LOGICAL_DISK_PROPERTIES, properties) + + @mock.patch.object(driver.DriverManager, '_list', autospec=True) + def test_driver_raid_logical_disk_properties_indexerror(self, _list_mock): + _list_mock.side_effect = IndexError + + properties = self.mgr.raid_logical_disk_properties(DRIVER2['name']) + + _list_mock.assert_called_once_with( + self.mgr, + '/v1/drivers/%s/raid/logical_disk_properties' % DRIVER2['name']) + self.assertEqual({}, properties) + @mock.patch.object(driver.DriverManager, 'update') def test_vendor_passthru_update(self, update_mock): # For now just mock the tests because vendor-passthru doesn't return diff --git a/ironicclient/tests/unit/v1/test_driver_shell.py b/ironicclient/tests/unit/v1/test_driver_shell.py index 4a45b66..d05902e 100644 --- a/ironicclient/tests/unit/v1/test_driver_shell.py +++ b/ironicclient/tests/unit/v1/test_driver_shell.py @@ -104,6 +104,30 @@ class DriverShellTest(utils.BaseTestCase): dict_value='Description', wrap=80) + @mock.patch('ironicclient.common.cliutils.print_dict') + def _test_do_driver_raid_logical_disk(self, print_dict_mock, wrap=0): + cli_mock = self.client_mock + cli_mock.driver.raid_logical_disk_properties.return_value = { + 'foo': 'bar'} + args = mock.MagicMock() + args.driver_name = 'driver_name' + args.wrap = wrap + + d_shell.do_driver_raid_logical_disk_properties(cli_mock, args) + + cli_mock.driver.raid_logical_disk_properties.assert_called_once_with( + "driver_name") + print_dict_mock.assert_called_with( + {'foo': 'bar'}, + dict_value='Description', + wrap=wrap) + + def test_do_driver_raid_logical_disk_default_wrap(self): + self._test_do_driver_raid_logical_disk() + + def test_do_driver_raid_logical_disk_with_wrap(self): + self._test_do_driver_raid_logical_disk(wrap=80) + def test_do_driver_show(self): client_mock = self.client_mock args = mock.MagicMock() diff --git a/ironicclient/tests/unit/v1/test_node.py b/ironicclient/tests/unit/v1/test_node.py index 34f93af..e7c5f63 100644 --- a/ironicclient/tests/unit/v1/test_node.py +++ b/ironicclient/tests/unit/v1/test_node.py @@ -16,6 +16,7 @@ import copy import tempfile import mock +import six import testtools from testtools.matchers import HasLength @@ -24,6 +25,10 @@ from ironicclient import exc from ironicclient.tests.unit import utils from ironicclient.v1 import node +if six.PY3: + import io + file = io.BytesIO + NODE1 = {'id': 123, 'uuid': '66666666-7777-8888-9999-000000000000', 'chassis_uuid': 'aaaaaaaa-1111-bbbb-2222-cccccccccccc', @@ -277,6 +282,13 @@ fake_responses = { NODE_STATES, ), }, + '/v1/nodes/%s/states/raid' % NODE1['uuid']: + { + 'PUT': ( + {}, + None, + ), + }, '/v1/nodes/%s/states/console' % NODE1['uuid']: { 'GET': ( @@ -786,6 +798,15 @@ class NodeManagerTest(testtools.TestCase): self.assertEqual(expect, self.api.calls) self.assertEqual('power on', power_state.target_power_state) + def test_set_target_raid_config(self): + self.mgr.set_target_raid_config( + NODE1['uuid'], {'fake': 'config'}) + + expect = [('PUT', '/v1/nodes/%s/states/raid' % NODE1['uuid'], + {}, + {'fake': 'config'})] + self.assertEqual(expect, self.api.calls) + def test_node_validate(self): ifaces = self.mgr.validate(NODE1['uuid']) expect = [ diff --git a/ironicclient/tests/unit/v1/test_node_shell.py b/ironicclient/tests/unit/v1/test_node_shell.py index 4545b92..8ff4ba9 100644 --- a/ironicclient/tests/unit/v1/test_node_shell.py +++ b/ironicclient/tests/unit/v1/test_node_shell.py @@ -58,7 +58,9 @@ class NodeShellTest(utils.BaseTestCase): 'updated_at', 'inspection_finished_at', 'inspection_started_at', - 'uuid'] + 'uuid', + 'raid_config', + 'target_raid_config'] act = actual.keys() self.assertEqual(sorted(exp), sorted(act)) @@ -352,6 +354,68 @@ class NodeShellTest(utils.BaseTestCase): def test_do_node_set_power_state_reboot(self): self._do_node_set_power_state_helper('reboot') + def test_do_node_set_target_raid_config_file(self): + contents = '{"raid": "config"}' + + with tempfile.NamedTemporaryFile(mode='w') as f: + f.write(contents) + f.flush() + + node_manager_mock = mock.MagicMock(spec=['set_target_raid_config']) + client_mock = mock.MagicMock(spec=['node'], node=node_manager_mock) + args = mock.MagicMock() + args.node = 'node_uuid' + args.target_raid_config = f.name + + n_shell.do_node_set_target_raid_config(client_mock, args) + node_manager_mock.set_target_raid_config.assert_called_once_with( + 'node_uuid', json.loads(contents)) + + def test_do_node_set_target_raid_config_string(self): + node_manager_mock = mock.MagicMock(spec=['set_target_raid_config']) + client_mock = mock.MagicMock(spec=['node'], node=node_manager_mock) + target_raid_config_string = ( + '{"logical_disks": [{"size_gb": 100, "raid_level": "1"}]}') + expected_target_raid_config_string = json.loads( + target_raid_config_string) + + args = mock.MagicMock(node='node', + target_raid_config=target_raid_config_string) + n_shell.do_node_set_target_raid_config(client_mock, args) + + node_manager_mock.set_target_raid_config.assert_called_once_with( + 'node', expected_target_raid_config_string) + + @mock.patch.object(n_shell, '_get_from_stdin', autospec=True) + def test_set_target_raid_config_stdin(self, stdin_read_mock): + node_manager_mock = mock.MagicMock(spec=['set_target_raid_config']) + client_mock = mock.MagicMock(spec=['node'], node=node_manager_mock) + target_raid_config_string = ( + '{"logical_disks": [{"size_gb": 100, "raid_level": "1"}]}') + stdin_read_mock.return_value = target_raid_config_string + args_mock = mock.MagicMock(node='node', + target_raid_config='-') + expected_target_raid_config_string = json.loads( + target_raid_config_string) + n_shell.do_node_set_target_raid_config(client_mock, args_mock) + stdin_read_mock.assert_called_once_with('target_raid_config') + client_mock.node.set_target_raid_config.assert_called_once_with( + 'node', expected_target_raid_config_string) + + @mock.patch.object(n_shell, '_get_from_stdin', autospec=True) + def test_set_target_raid_config_stdin_exception(self, stdin_read_mock): + client_mock = mock.MagicMock() + stdin_read_mock.side_effect = exc.InvalidAttribute('bad') + args_mock = mock.MagicMock(node='node', + target_raid_config='-') + + self.assertRaises(exc.InvalidAttribute, + n_shell.do_node_set_target_raid_config, + client_mock, args_mock) + + stdin_read_mock.assert_called_once_with('target_raid_config') + self.assertFalse(client_mock.set_target_raid_config.called) + def test_do_node_validate(self): client_mock = mock.MagicMock() args = mock.MagicMock() @@ -941,29 +1005,29 @@ class NodeShellLocalTest(utils.BaseTestCase): self.assertRaises(exc.InvalidAttribute, n_shell._get_from_stdin, desc) mock_stdin.read.assert_called_once_with() - def test__handle_clean_steps_arg(self): + def test__handle_json_or_file_arg(self): cleansteps = '[{"step": "upgrade", "interface": "deploy"}]' - steps = n_shell._handle_clean_steps_arg(cleansteps) + steps = n_shell._handle_json_or_file_arg(cleansteps) self.assertEqual(json.loads(cleansteps), steps) - def test__handle_clean_steps_arg_bad_json(self): + def test__handle_json_or_file_arg_bad_json(self): cleansteps = 'foo' self.assertRaisesRegex(exc.InvalidAttribute, - 'For clean steps', - n_shell._handle_clean_steps_arg, cleansteps) + 'For JSON', + n_shell._handle_json_or_file_arg, cleansteps) - def test__handle_clean_steps_arg_file(self): + def test__handle_json_or_file_arg_file(self): contents = '[{"step": "upgrade", "interface": "deploy"}]' with tempfile.NamedTemporaryFile(mode='w') as f: f.write(contents) f.flush() - steps = n_shell._handle_clean_steps_arg(f.name) + steps = n_shell._handle_json_or_file_arg(f.name) self.assertEqual(json.loads(contents), steps) @mock.patch.object(__builtin__, 'open', autospec=True) - def test__handle_clean_steps_arg_file_fail(self, mock_open): + def test__handle_json_or_file_arg_file_fail(self, mock_open): mock_file_object = mock.MagicMock() mock_file_handle = mock.MagicMock() mock_file_handle.__enter__.return_value = mock_file_object @@ -973,6 +1037,6 @@ class NodeShellLocalTest(utils.BaseTestCase): with tempfile.NamedTemporaryFile(mode='w') as f: self.assertRaisesRegex(exc.InvalidAttribute, "from file", - n_shell._handle_clean_steps_arg, f.name) + n_shell._handle_json_or_file_arg, f.name) mock_open.assert_called_once_with(f.name, 'r') mock_file_object.read.assert_called_once_with() diff --git a/ironicclient/v1/driver.py b/ironicclient/v1/driver.py index 5897fa4..5cb94b3 100644 --- a/ironicclient/v1/driver.py +++ b/ironicclient/v1/driver.py @@ -43,6 +43,25 @@ class DriverManager(base.Manager): def properties(self, driver_name): return self._get(resource_id='%s/properties' % driver_name).to_dict() + def raid_logical_disk_properties(self, driver_name): + """Returns the RAID logical disk properties for the driver. + + :param driver_name: Name of the driver. + :returns: A dictionary containing the properties that can be mentioned + for RAID logical disks and a textual description for them. It + returns an empty dictionary on error. + """ + info = None + try: + info = self._list( + '/v1/drivers/%s/raid/logical_disk_properties' % driver_name)[0] + except IndexError: + pass + + if info: + return info.to_dict() + return {} + def vendor_passthru(self, driver_name, method, args=None, http_method=None): """Issue requests for vendor-specific actions on a given driver. diff --git a/ironicclient/v1/driver_shell.py b/ironicclient/v1/driver_shell.py index 6e6683e..1cb5690 100644 --- a/ironicclient/v1/driver_shell.py +++ b/ironicclient/v1/driver_shell.py @@ -62,6 +62,22 @@ def do_driver_properties(cc, args): dict_value='Description') +@cliutils.arg('driver_name', metavar='', + help="Name of the driver.") +@cliutils.arg('--wrap', dest='wrap', metavar='', + type=int, default=0, + help=('Wrap the output to a specified length. ' + 'Positive number can realize wrap functionality. ' + '0 is default for disabled.')) +def do_driver_raid_logical_disk_properties(cc, args): + """Get RAID logical disk properties for a driver.""" + properties = cc.driver.raid_logical_disk_properties(args.driver_name) + cliutils.print_dict( + properties, + wrap=args.wrap, + dict_value='Description') + + @cliutils.arg('driver_name', metavar='', help='Name of the driver.') diff --git a/ironicclient/v1/node.py b/ironicclient/v1/node.py index d6b84e5..44e8245 100644 --- a/ironicclient/v1/node.py +++ b/ironicclient/v1/node.py @@ -255,6 +255,17 @@ class NodeManager(base.CreateManager): target = {'target': _power_states.get(state, state)} return self.update(path, target, http_method='PUT') + def set_target_raid_config(self, node_ident, target_raid_config): + """Sets target_raid_config for a node. + + :param node_ident: Node identifier + :param target_raid_config: A dictionary with the target RAID + configuration; may be empty. + :returns: status of the request + """ + path = "%s/states/raid" % node_ident + return self.update(path, target_raid_config, http_method='PUT') + def validate(self, node_uuid): path = "%s/validate" % node_uuid return self.get(path) diff --git a/ironicclient/v1/node_shell.py b/ironicclient/v1/node_shell.py index 1850124..d714615 100644 --- a/ironicclient/v1/node_shell.py +++ b/ironicclient/v1/node_shell.py @@ -50,31 +50,31 @@ def _get_from_stdin(info_desc): return info -def _handle_clean_steps_arg(clean_steps): - """Attempts to read clean steps argument. +def _handle_json_or_file_arg(json_arg): + """Attempts to read JSON argument from file or string. - :param clean_steps: May be a file name containing the clean steps, or - a JSON string representing the clean steps. - :returns: A list of dictionaries representing clean steps. + :param json_arg: May be a file name containing the JSON, or + a JSON string. + :returns: A list or dictionary parsed from JSON. :raises: InvalidAttribute if the argument cannot be parsed. """ - if os.path.isfile(clean_steps): + if os.path.isfile(json_arg): try: - with open(clean_steps, 'r') as f: - clean_steps = f.read().strip() + with open(json_arg, 'r') as f: + json_arg = f.read().strip() except Exception as e: - err = _("Cannot get clean steps from file '%(file)s'. " - "Error: %(err)s") % {'err': e, 'file': clean_steps} + err = _("Cannot get JSON from file '%(file)s'. " + "Error: %(err)s") % {'err': e, 'file': json_arg} raise exc.InvalidAttribute(err) try: - clean_steps = json.loads(clean_steps) + json_arg = json.loads(json_arg) except ValueError as e: - err = (_("For clean steps: '%(steps)s', error: '%(err)s'") % - {'err': e, 'steps': clean_steps}) + err = (_("For JSON: '%(string)s', error: '%(err)s'") % + {'err': e, 'string': json_arg}) raise exc.InvalidAttribute(err) - return clean_steps + return json_arg @cliutils.arg( @@ -422,6 +422,30 @@ def do_node_set_power_state(cc, args): cc.node.set_power_state(args.node, args.power_state) +@cliutils.arg('node', metavar='', help="Name or UUID of the node.") +@cliutils.arg( + 'target_raid_config', + metavar='', + help=("A file containing JSON data of the desired RAID configuration. " + "Use '-' to read the contents from standard input. " + "It also accepts the valid json string as input if " + "file/standard input are not used for providing input. " + "The input can be an empty dictionary too which " + "unsets the node.target_raid_config on the node.")) +def do_node_set_target_raid_config(cc, args): + """Set target RAID config on a node.""" + target_raid_config = args.target_raid_config + if not target_raid_config: + raise exc.InvalidAttribute( + _("target RAID configuration not provided")) + + if target_raid_config == '-': + target_raid_config = _get_from_stdin('target_raid_config') + target_raid_config = _handle_json_or_file_arg(target_raid_config) + + cc.node.set_target_raid_config(args.node, target_raid_config) + + @cliutils.arg('node', metavar='', help="Name or UUID of the node.") @cliutils.arg( 'provision_state', @@ -466,7 +490,7 @@ def do_node_set_provision_state(cc, args): if args.clean_steps == '-': clean_steps = _get_from_stdin('clean steps') if clean_steps: - clean_steps = _handle_clean_steps_arg(clean_steps) + clean_steps = _handle_json_or_file_arg(clean_steps) cc.node.set_provision_state(args.node, args.provision_state, configdrive=args.config_drive, cleansteps=clean_steps) diff --git a/ironicclient/v1/resource_fields.py b/ironicclient/v1/resource_fields.py index 91c0fb1..f5f9620 100644 --- a/ironicclient/v1/resource_fields.py +++ b/ironicclient/v1/resource_fields.py @@ -57,9 +57,11 @@ class Resource(object): 'properties': 'Properties', 'provision_state': 'Provisioning State', 'provision_updated_at': 'Provision Updated At', + 'raid_config': 'Current RAID configuration', 'reservation': 'Reservation', 'target_power_state': 'Target Power State', 'target_provision_state': 'Target Provision State', + 'target_raid_config': 'Target RAID configuration', 'updated_at': 'Updated At', 'uuid': 'UUID', } @@ -139,9 +141,11 @@ NODE_DETAILED_RESOURCE = Resource( 'properties', 'provision_state', 'provision_updated_at', + 'raid_config', 'reservation', 'target_power_state', 'target_provision_state', + 'target_raid_config', 'updated_at', 'inspection_finished_at', 'inspection_started_at', @@ -159,6 +163,8 @@ NODE_DETAILED_RESOURCE = Resource( 'extra', 'instance_info', 'properties', + 'raid_config', + 'target_raid_config', ]) NODE_RESOURCE = Resource( ['uuid', diff --git a/releasenotes/notes/raid_CLI_support-7e816ccd0fb31d2b.yaml b/releasenotes/notes/raid_CLI_support-7e816ccd0fb31d2b.yaml new file mode 100644 index 0000000..71219ba --- /dev/null +++ b/releasenotes/notes/raid_CLI_support-7e816ccd0fb31d2b.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + Adds support for RAID configuration: + + * Ability to set target_raid_config for a node using + node-set-target-raid-config. + + * Displays target_raid_config and raid_config in + node-show. + + * Displays logical disk properties for a driver using + driver-raid-logical-disk-properties. + + API version 1.15 should be used with these features, as + RAID configuration was added in 1.12 and manual cleaning + (used to trigger RAID configuration) was added in 1.15.