Use InstanceId as unique attribute identifier rather than name
AttributeName just happens to be unique for everything under DCIM_BIOS* but this is not a guarantee. Using InstanceId instead which is guaranteed to be unique. Discussion on the list http://lists.openstack.org/pipermail/openstack-dev/2016-September/103602.html Closes-Bug: #1635419 Signed-off-by: Anish Bhatt <anish.bhatt@salesforce.com> Change-Id: I93c0f7919ecfd77642634d1d0addfe8a79aa6f57
This commit is contained in:
@@ -121,9 +121,12 @@ class DRACClient(object):
|
||||
return self._boot_mgmt.change_boot_device_order(boot_mode,
|
||||
boot_device_list)
|
||||
|
||||
def list_bios_settings(self):
|
||||
def list_bios_settings(self, by_name=True):
|
||||
"""List the BIOS configuration settings
|
||||
|
||||
:param by_name: Controls whether returned dictionary uses BIOS
|
||||
attribute name as key. If set to False, instance_id
|
||||
will be used.
|
||||
:returns: a dictionary with the BIOS settings using its name as the
|
||||
key. The attributes are either BIOSEnumerableAttribute,
|
||||
BIOSStringAttribute or BIOSIntegerAttribute objects.
|
||||
@@ -132,7 +135,7 @@ class DRACClient(object):
|
||||
:raises: DRACOperationFailed on error reported back by the DRAC
|
||||
interface
|
||||
"""
|
||||
return self._bios_cfg.list_bios_settings()
|
||||
return self._bios_cfg.list_bios_settings(by_name)
|
||||
|
||||
def set_bios_settings(self, settings):
|
||||
"""Sets the BIOS configuration
|
||||
|
@@ -261,16 +261,19 @@ class BootManagement(object):
|
||||
class BIOSAttribute(object):
|
||||
"""Generic BIOS attribute class"""
|
||||
|
||||
def __init__(self, name, current_value, pending_value, read_only):
|
||||
def __init__(self, name, instance_id, current_value, pending_value,
|
||||
read_only):
|
||||
"""Creates BIOSAttribute object
|
||||
|
||||
:param name: name of the BIOS attribute
|
||||
:param instance_id: opaque and unique identifier of the BIOS attribute
|
||||
:param current_value: current value of the BIOS attribute
|
||||
:param pending_value: pending value of the BIOS attribute, reflecting
|
||||
an unprocessed change (eg. config job not completed)
|
||||
:param read_only: indicates whether this BIOS attribute can be changed
|
||||
"""
|
||||
self.name = name
|
||||
self.instance_id = instance_id
|
||||
self.current_value = current_value
|
||||
self.pending_value = pending_value
|
||||
self.read_only = read_only
|
||||
@@ -287,6 +290,8 @@ class BIOSAttribute(object):
|
||||
|
||||
name = utils.get_wsman_resource_attr(
|
||||
bios_attr_xml, namespace, 'AttributeName')
|
||||
instance_id = utils.get_wsman_resource_attr(
|
||||
bios_attr_xml, namespace, 'InstanceID')
|
||||
current_value = utils.get_wsman_resource_attr(
|
||||
bios_attr_xml, namespace, 'CurrentValue', nullable=True)
|
||||
pending_value = utils.get_wsman_resource_attr(
|
||||
@@ -294,7 +299,8 @@ class BIOSAttribute(object):
|
||||
read_only = utils.get_wsman_resource_attr(
|
||||
bios_attr_xml, namespace, 'IsReadOnly')
|
||||
|
||||
return cls(name, current_value, pending_value, (read_only == 'true'))
|
||||
return cls(name, instance_id, current_value, pending_value,
|
||||
(read_only == 'true'))
|
||||
|
||||
|
||||
class BIOSEnumerableAttribute(BIOSAttribute):
|
||||
@@ -302,8 +308,8 @@ class BIOSEnumerableAttribute(BIOSAttribute):
|
||||
|
||||
namespace = uris.DCIM_BIOSEnumeration
|
||||
|
||||
def __init__(self, name, current_value, pending_value, read_only,
|
||||
possible_values):
|
||||
def __init__(self, name, instance_id, current_value, pending_value,
|
||||
read_only, possible_values):
|
||||
"""Creates BIOSEnumerableAttribute object
|
||||
|
||||
:param name: name of the BIOS attribute
|
||||
@@ -314,7 +320,8 @@ class BIOSEnumerableAttribute(BIOSAttribute):
|
||||
:param possible_values: list containing the allowed values for the BIOS
|
||||
attribute
|
||||
"""
|
||||
super(BIOSEnumerableAttribute, self).__init__(name, current_value,
|
||||
super(BIOSEnumerableAttribute, self).__init__(name, instance_id,
|
||||
current_value,
|
||||
pending_value, read_only)
|
||||
self.possible_values = possible_values
|
||||
|
||||
@@ -327,9 +334,9 @@ class BIOSEnumerableAttribute(BIOSAttribute):
|
||||
in utils.find_xml(bios_attr_xml, 'PossibleValues',
|
||||
cls.namespace, find_all=True)]
|
||||
|
||||
return cls(bios_attr.name, bios_attr.current_value,
|
||||
bios_attr.pending_value, bios_attr.read_only,
|
||||
possible_values)
|
||||
return cls(bios_attr.name, bios_attr.instance_id,
|
||||
bios_attr.current_value, bios_attr.pending_value,
|
||||
bios_attr.read_only, possible_values)
|
||||
|
||||
def validate(self, new_value):
|
||||
"""Validates new value"""
|
||||
@@ -348,8 +355,8 @@ class BIOSStringAttribute(BIOSAttribute):
|
||||
|
||||
namespace = uris.DCIM_BIOSString
|
||||
|
||||
def __init__(self, name, current_value, pending_value, read_only,
|
||||
min_length, max_length, pcre_regex):
|
||||
def __init__(self, name, instance_id, current_value, pending_value,
|
||||
read_only, min_length, max_length, pcre_regex):
|
||||
"""Creates BIOSStringAttribute object
|
||||
|
||||
:param name: name of the BIOS attribute
|
||||
@@ -362,8 +369,9 @@ class BIOSStringAttribute(BIOSAttribute):
|
||||
:param pcre_regex: is a PCRE compatible regular expression that the
|
||||
string must match
|
||||
"""
|
||||
super(BIOSStringAttribute, self).__init__(name, current_value,
|
||||
pending_value, read_only)
|
||||
super(BIOSStringAttribute, self).__init__(name, instance_id,
|
||||
current_value, pending_value,
|
||||
read_only)
|
||||
self.min_length = min_length
|
||||
self.max_length = max_length
|
||||
self.pcre_regex = pcre_regex
|
||||
@@ -380,9 +388,9 @@ class BIOSStringAttribute(BIOSAttribute):
|
||||
pcre_regex = utils.get_wsman_resource_attr(
|
||||
bios_attr_xml, cls.namespace, 'ValueExpression', nullable=True)
|
||||
|
||||
return cls(bios_attr.name, bios_attr.current_value,
|
||||
bios_attr.pending_value, bios_attr.read_only,
|
||||
min_length, max_length, pcre_regex)
|
||||
return cls(bios_attr.name, bios_attr.instance_id,
|
||||
bios_attr.current_value, bios_attr.pending_value,
|
||||
bios_attr.read_only, min_length, max_length, pcre_regex)
|
||||
|
||||
def validate(self, new_value):
|
||||
"""Validates new value"""
|
||||
@@ -403,8 +411,8 @@ class BIOSIntegerAttribute(BIOSAttribute):
|
||||
|
||||
namespace = uris.DCIM_BIOSInteger
|
||||
|
||||
def __init__(self, name, current_value, pending_value, read_only,
|
||||
lower_bound, upper_bound):
|
||||
def __init__(self, name, instance_id, current_value, pending_value,
|
||||
read_only, lower_bound, upper_bound):
|
||||
"""Creates BIOSIntegerAttribute object
|
||||
|
||||
:param name: name of the BIOS attribute
|
||||
@@ -415,7 +423,8 @@ class BIOSIntegerAttribute(BIOSAttribute):
|
||||
:param lower_bound: minimum value for the BIOS attribute
|
||||
:param upper_bound: maximum value for the BOIS attribute
|
||||
"""
|
||||
super(BIOSIntegerAttribute, self).__init__(name, current_value,
|
||||
super(BIOSIntegerAttribute, self).__init__(name, instance_id,
|
||||
current_value,
|
||||
pending_value, read_only)
|
||||
self.lower_bound = lower_bound
|
||||
self.upper_bound = upper_bound
|
||||
@@ -435,9 +444,9 @@ class BIOSIntegerAttribute(BIOSAttribute):
|
||||
if bios_attr.pending_value:
|
||||
bios_attr.pending_value = int(bios_attr.pending_value)
|
||||
|
||||
return cls(bios_attr.name, bios_attr.current_value,
|
||||
bios_attr.pending_value, bios_attr.read_only,
|
||||
int(lower_bound), int(upper_bound))
|
||||
return cls(bios_attr.name, bios_attr.instance_id,
|
||||
bios_attr.current_value, bios_attr.pending_value,
|
||||
bios_attr.read_only, int(lower_bound), int(upper_bound))
|
||||
|
||||
def validate(self, new_value):
|
||||
"""Validates new value"""
|
||||
@@ -462,9 +471,11 @@ class BIOSConfiguration(object):
|
||||
"""
|
||||
self.client = client
|
||||
|
||||
def list_bios_settings(self):
|
||||
def list_bios_settings(self, by_name=True):
|
||||
"""List the BIOS configuration settings
|
||||
|
||||
:param by_name: Controls whether returned dictionary uses BIOS
|
||||
attribute name or instance_id as key.
|
||||
:returns: a dictionary with the BIOS settings using its name as the
|
||||
key. The attributes are either BIOSEnumerableAttribute,
|
||||
BIOSStringAttribute or BIOSIntegerAttribute objects.
|
||||
@@ -479,7 +490,7 @@ class BIOSConfiguration(object):
|
||||
(uris.DCIM_BIOSString, BIOSStringAttribute),
|
||||
(uris.DCIM_BIOSInteger, BIOSIntegerAttribute)]
|
||||
for (namespace, attr_cls) in namespaces:
|
||||
attribs = self._get_config(namespace, attr_cls)
|
||||
attribs = self._get_config(namespace, attr_cls, by_name)
|
||||
if not set(result).isdisjoint(set(attribs)):
|
||||
raise exceptions.DRACOperationFailed(
|
||||
drac_messages=('Colliding attributes %r' % (
|
||||
@@ -487,7 +498,7 @@ class BIOSConfiguration(object):
|
||||
result.update(attribs)
|
||||
return result
|
||||
|
||||
def _get_config(self, resource, attr_cls):
|
||||
def _get_config(self, resource, attr_cls, by_name):
|
||||
result = {}
|
||||
|
||||
doc = self.client.enumerate(resource)
|
||||
@@ -495,7 +506,10 @@ class BIOSConfiguration(object):
|
||||
|
||||
for item in items:
|
||||
attribute = attr_cls.parse(item)
|
||||
result[attribute.name] = attribute
|
||||
if by_name:
|
||||
result[attribute.name] = attribute
|
||||
else:
|
||||
result[attribute.instance_id] = attribute
|
||||
|
||||
return result
|
||||
|
||||
@@ -520,7 +534,14 @@ class BIOSConfiguration(object):
|
||||
:raises: InvalidParameterValue on invalid BIOS attribute
|
||||
"""
|
||||
|
||||
current_settings = self.list_bios_settings()
|
||||
current_settings = self.list_bios_settings(by_name=True)
|
||||
# BIOS settings are returned as dict indexed by InstanceID.
|
||||
# However DCIM_BIOSService requires attribute name, not instance id
|
||||
# so recreate this as a dict indexed by attribute name
|
||||
# TODO(anish) : Enable this code if/when by_name gets deprecated
|
||||
# bios_settings = self.list_bios_settings(by_name=False)
|
||||
# current_settings = dict((value.name, value)
|
||||
# for key, value in bios_settings.items())
|
||||
unknown_keys = set(new_settings) - set(current_settings)
|
||||
if unknown_keys:
|
||||
msg = ('Unknown BIOS attributes found: %(unknown_keys)r' %
|
||||
|
@@ -199,15 +199,17 @@ class ClientBIOSConfigurationTestCase(base.BaseTest):
|
||||
**test_utils.FAKE_ENDPOINT)
|
||||
|
||||
@requests_mock.Mocker()
|
||||
def test_list_bios_settings(self, mock_requests):
|
||||
def test_list_bios_settings_by_instance_id(self, mock_requests):
|
||||
expected_enum_attr = bios.BIOSEnumerableAttribute(
|
||||
name='MemTest',
|
||||
instance_id='BIOS.Setup.1-1:MemTest',
|
||||
read_only=False,
|
||||
current_value='Disabled',
|
||||
pending_value=None,
|
||||
possible_values=['Enabled', 'Disabled'])
|
||||
expected_string_attr = bios.BIOSStringAttribute(
|
||||
name='SystemModelName',
|
||||
instance_id='BIOS.Setup.1-1:SystemModelName',
|
||||
read_only=True,
|
||||
current_value='PowerEdge R320',
|
||||
pending_value=None,
|
||||
@@ -216,6 +218,7 @@ class ClientBIOSConfigurationTestCase(base.BaseTest):
|
||||
pcre_regex=None)
|
||||
expected_integer_attr = bios.BIOSIntegerAttribute(
|
||||
name='Proc1NumCores',
|
||||
instance_id='BIOS.Setup.1-1:Proc1NumCores',
|
||||
read_only=True,
|
||||
current_value=8,
|
||||
pending_value=None,
|
||||
@@ -229,7 +232,57 @@ class ClientBIOSConfigurationTestCase(base.BaseTest):
|
||||
{'text': test_utils.BIOSEnumerations[
|
||||
uris.DCIM_BIOSInteger]['ok']}])
|
||||
|
||||
bios_settings = self.drac_client.list_bios_settings()
|
||||
bios_settings = self.drac_client.list_bios_settings(by_name=False)
|
||||
|
||||
self.assertEqual(103, len(bios_settings))
|
||||
# enumerable attribute
|
||||
self.assertIn('BIOS.Setup.1-1:MemTest', bios_settings)
|
||||
self.assertEqual(expected_enum_attr, bios_settings[
|
||||
'BIOS.Setup.1-1:MemTest'])
|
||||
# string attribute
|
||||
self.assertIn('BIOS.Setup.1-1:SystemModelName', bios_settings)
|
||||
self.assertEqual(expected_string_attr,
|
||||
bios_settings['BIOS.Setup.1-1:SystemModelName'])
|
||||
# integer attribute
|
||||
self.assertIn('BIOS.Setup.1-1:Proc1NumCores', bios_settings)
|
||||
self.assertEqual(expected_integer_attr, bios_settings[
|
||||
'BIOS.Setup.1-1:Proc1NumCores'])
|
||||
|
||||
@requests_mock.Mocker()
|
||||
def test_list_bios_settings_by_name(self, mock_requests):
|
||||
expected_enum_attr = bios.BIOSEnumerableAttribute(
|
||||
name='MemTest',
|
||||
instance_id='BIOS.Setup.1-1:MemTest',
|
||||
read_only=False,
|
||||
current_value='Disabled',
|
||||
pending_value=None,
|
||||
possible_values=['Enabled', 'Disabled'])
|
||||
expected_string_attr = bios.BIOSStringAttribute(
|
||||
name='SystemModelName',
|
||||
instance_id='BIOS.Setup.1-1:SystemModelName',
|
||||
read_only=True,
|
||||
current_value='PowerEdge R320',
|
||||
pending_value=None,
|
||||
min_length=0,
|
||||
max_length=32,
|
||||
pcre_regex=None)
|
||||
expected_integer_attr = bios.BIOSIntegerAttribute(
|
||||
name='Proc1NumCores',
|
||||
instance_id='BIOS.Setup.1-1:Proc1NumCores',
|
||||
read_only=True,
|
||||
current_value=8,
|
||||
pending_value=None,
|
||||
lower_bound=0,
|
||||
upper_bound=65535)
|
||||
mock_requests.post('https://1.2.3.4:443/wsman', [
|
||||
{'text': test_utils.BIOSEnumerations[
|
||||
uris.DCIM_BIOSEnumeration]['ok']},
|
||||
{'text': test_utils.BIOSEnumerations[
|
||||
uris.DCIM_BIOSString]['ok']},
|
||||
{'text': test_utils.BIOSEnumerations[
|
||||
uris.DCIM_BIOSInteger]['ok']}])
|
||||
|
||||
bios_settings = self.drac_client.list_bios_settings(by_name=True)
|
||||
|
||||
self.assertEqual(103, len(bios_settings))
|
||||
# enumerable attribute
|
||||
@@ -244,7 +297,8 @@ class ClientBIOSConfigurationTestCase(base.BaseTest):
|
||||
self.assertEqual(expected_integer_attr, bios_settings['Proc1NumCores'])
|
||||
|
||||
@requests_mock.Mocker()
|
||||
def test_list_bios_settings_with_colliding_attrs(self, mock_requests):
|
||||
def test_list_bios_settings_by_name_with_colliding_attrs(
|
||||
self, mock_requests):
|
||||
mock_requests.post('https://1.2.3.4:443/wsman', [
|
||||
{'text': test_utils.BIOSEnumerations[
|
||||
uris.DCIM_BIOSEnumeration]['ok']},
|
||||
@@ -254,7 +308,7 @@ class ClientBIOSConfigurationTestCase(base.BaseTest):
|
||||
uris.DCIM_BIOSInteger]['ok']}])
|
||||
|
||||
self.assertRaises(exceptions.DRACOperationFailed,
|
||||
self.drac_client.list_bios_settings)
|
||||
self.drac_client.list_bios_settings, by_name=True)
|
||||
|
||||
@requests_mock.Mocker()
|
||||
@mock.patch.object(dracclient.client.WSManClient, 'invoke',
|
||||
|
Reference in New Issue
Block a user