Skip to content

Commit

Permalink
Use InstanceId as unique attribute identifier rather than name
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@anish
anish committed Oct 27, 2016
1 parent 0cfeff4 commit 5e6b55d
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 32 deletions.
7 changes: 5 additions & 2 deletions dracclient/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,12 @@ def change_boot_device_order(self, boot_mode, boot_device_list):
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.
Expand All @@ -132,7 +135,7 @@ def list_bios_settings(self):
: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
Expand Down
73 changes: 47 additions & 26 deletions dracclient/resources/bios.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,16 +261,19 @@ def _get_boot_device_attr(self, drac_boot_device, attr_name):
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
Expand All @@ -287,23 +290,26 @@ def parse(cls, namespace, bios_attr_xml):

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(
bios_attr_xml, namespace, 'PendingValue', nullable=True)
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):
"""Enumerable BIOS attribute class"""

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
Expand All @@ -314,7 +320,8 @@ def __init__(self, name, current_value, pending_value, read_only,
: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

Expand All @@ -327,9 +334,9 @@ def parse(cls, bios_attr_xml):
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"""
Expand All @@ -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
Expand All @@ -362,8 +369,9 @@ def __init__(self, name, current_value, pending_value, read_only,
: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
Expand All @@ -380,9 +388,9 @@ def parse(cls, bios_attr_xml):
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"""
Expand All @@ -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
Expand All @@ -415,7 +423,8 @@ def __init__(self, name, current_value, pending_value, read_only,
: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
Expand All @@ -435,9 +444,9 @@ def parse(cls, bios_attr_xml):
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"""
Expand All @@ -462,9 +471,11 @@ def __init__(self, client):
"""
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.
Expand All @@ -479,23 +490,26 @@ def list_bios_settings(self):
(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' % (
set(result) & set(attribs))))
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)
items = doc.find('.//{%s}Items' % wsman.NS_WSMAN)

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

Expand All @@ -520,7 +534,14 @@ def set_bios_settings(self, new_settings):
: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' %
Expand Down
62 changes: 58 additions & 4 deletions dracclient/tests/test_bios.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,17 @@ def setUp(self):
**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,
Expand All @@ -216,6 +218,7 @@ def test_list_bios_settings(self, mock_requests):
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,
Expand All @@ -229,7 +232,57 @@ def test_list_bios_settings(self, mock_requests):
{'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
Expand All @@ -244,7 +297,8 @@ def test_list_bios_settings(self, mock_requests):
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']},
Expand All @@ -254,7 +308,7 @@ def test_list_bios_settings_with_colliding_attrs(self, mock_requests):
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',
Expand Down

0 comments on commit 5e6b55d

Please sign in to comment.