openstack
/
cinder
Code Issues Proposed changes

NetApp ONTAP: Fixed get_ontap_version 

Changed the method get_ontap_version, adjusting the code to return
a tuple (generation, major, minor) instead of a string, to avoid
problems when comparing strings in alphabetical order (e.g. '9.5' >
'9.10').

Change-Id: I2305b49ab7d2a1c4d9affd55a4b5b746a1ae4dce
Closes-Bug: #1955057
This commit is contained in:
Nahim Alves de Souza 2021-11-18 19:17:02 +00:00
parent 0d9076383c
commit 3b13eab894
5 changed files with 55 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_base.py
View File

@ -89,10 +89,10 @@ class NetAppBaseClientTestCase(test.TestCase):
self.assertIsNone(self.client.check_is_naelement(element))
self.assertRaises(ValueError, self.client.check_is_naelement, None)
@ddt.data({'ontap_version': '9.4', 'space_reservation': 'true'},
{'ontap_version': '9.4', 'space_reservation': 'false'},
{'ontap_version': '9.6', 'space_reservation': 'true'},
{'ontap_version': '9.6', 'space_reservation': 'false'})
@ddt.data({'ontap_version': (9, 4, 0), 'space_reservation': 'true'},
{'ontap_version': (9, 4, 0), 'space_reservation': 'false'},
{'ontap_version': (9, 6, 0), 'space_reservation': 'true'},
{'ontap_version': (9, 6, 0), 'space_reservation': 'false'})
@ddt.unpack
def test_create_lun(self, ontap_version, space_reservation):
expected_path = '/vol/%s/%s' % (self.fake_volume, self.fake_lun)
@ -108,7 +108,7 @@ class NetAppBaseClientTestCase(test.TestCase):
client_base.Client, '_validate_qos_policy_group')
initial_size = self.fake_size
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
initial_size = fake.MAX_SIZE_FOR_A_LUN
expected_space_reservation = 'false'
@ -131,20 +131,20 @@ class NetAppBaseClientTestCase(test.TestCase):
self.connection.invoke_successfully.assert_called_with(
mock.ANY, True)
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
mock_resize_lun.assert_called_once_with(
expected_path, self.fake_size)
if ontap_version < '9.5' and space_reservation == 'true':
if ontap_version < (9, 5, 0) and space_reservation == 'true':
mock_set_space_reservation.assert_called_once_with(
expected_path, True)
else:
mock_set_space_reservation.assert_not_called()
@ddt.data({'ontap_version': '9.4', 'space_reservation': 'true'},
{'ontap_version': '9.4', 'space_reservation': 'false'},
{'ontap_version': '9.6', 'space_reservation': 'true'},
{'ontap_version': '9.6', 'space_reservation': 'false'})
@ddt.data({'ontap_version': (9, 4, 0), 'space_reservation': 'true'},
{'ontap_version': (9, 4, 0), 'space_reservation': 'false'},
{'ontap_version': (9, 6, 0), 'space_reservation': 'true'},
{'ontap_version': (9, 6, 0), 'space_reservation': 'false'})
@ddt.unpack
def test_create_lun_exact_size(self, ontap_version, space_reservation):
expected_path = '/vol/%s/%s' % (self.fake_volume, self.fake_lun)
@ -161,7 +161,7 @@ class NetAppBaseClientTestCase(test.TestCase):
client_base.Client, '_validate_qos_policy_group')
initial_size = self.fake_size
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
initial_size = fake.MAX_SIZE_FOR_A_LUN
expected_space_reservation = 'false'
@ -185,20 +185,20 @@ class NetAppBaseClientTestCase(test.TestCase):
self.connection.invoke_successfully.assert_called_with(
mock.ANY, True)
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
mock_resize_lun.assert_called_once_with(
expected_path, self.fake_size)
if ontap_version < '9.5' and space_reservation == 'true':
if ontap_version < (9, 5, 0) and space_reservation == 'true':
mock_set_space_reservation.assert_called_once_with(
expected_path, True)
else:
mock_set_space_reservation.assert_not_called()
@ddt.data({'ontap_version': '9.4', 'space_reservation': 'true'},
{'ontap_version': '9.4', 'space_reservation': 'false'},
{'ontap_version': '9.6', 'space_reservation': 'true'},
{'ontap_version': '9.6', 'space_reservation': 'false'})
@ddt.data({'ontap_version': (9, 4, 0), 'space_reservation': 'true'},
{'ontap_version': (9, 4, 0), 'space_reservation': 'false'},
{'ontap_version': (9, 6, 0), 'space_reservation': 'true'},
{'ontap_version': (9, 6, 0), 'space_reservation': 'false'})
@ddt.unpack
def test_create_lun_with_qos_policy_group_name(
self, ontap_version, space_reservation):
@ -218,7 +218,7 @@ class NetAppBaseClientTestCase(test.TestCase):
client_base.Client, '_validate_qos_policy_group')
initial_size = self.fake_size
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
initial_size = fake.MAX_SIZE_FOR_A_LUN
expected_space_reservation = 'false'
@ -245,11 +245,11 @@ class NetAppBaseClientTestCase(test.TestCase):
self.connection.invoke_successfully.assert_called_with(
mock.ANY, True)
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
mock_resize_lun.assert_called_once_with(
expected_path, self.fake_size)
if ontap_version < '9.5' and space_reservation == 'true':
if ontap_version < (9, 5, 0) and space_reservation == 'true':
mock_set_space_reservation.assert_called_once_with(
expected_path, True)
else:
@ -258,19 +258,20 @@ class NetAppBaseClientTestCase(test.TestCase):
def test_get_ontap_version(self):
version_response = netapp_api.NaElement(
fake.SYSTEM_GET_VERSION_RESPONSE)
self.connection.invoke_successfully.return_value = version_response
self.connection.invoke_successfully.return_value = (
version_response)
result = self.client.get_ontap_version(cached=False)
self.assertEqual(('9.6'), result)
self.assertEqual((9, 6, 0), result)
def test_get_ontap_version_cached(self):
self.connection.get_ontap_version.return_value = '9.6'
self.connection.get_ontap_version.return_value = (9, 6, 0)
result = self.client.get_ontap_version()
self.connection.get_ontap_version.assert_called_once_with()
self.assertEqual(('9.6'), result)
self.assertEqual((9, 6, 0), result)
def test_set_lun_space_reservation(self):
path = '/vol/%s/%s' % (self.fake_volume, self.fake_lun)
@ -287,7 +288,7 @@ class NetAppBaseClientTestCase(test.TestCase):
self.connection.invoke_successfully.assert_called_once_with(
mock.ANY, True)
@ddt.data('9.4', '9.6')
@ddt.data((9, 4, 0), (9, 6, 0))
def test_create_lun_raises_on_failure(self, ontap_version):
self.connection.invoke_successfully = mock.Mock(
side_effect=netapp_api.NaApiError)

18
cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py
View File

@ -1213,7 +1213,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
mock_extend_volume.assert_called_once_with(
fake.VOLUME, new_size, fake.QOS_POLICY_GROUP_NAME)
@ddt.data('9.4', '9.6')
@ddt.data((9, 4, 0), (9, 6, 0))
def test__extend_volume_direct(self, ontap_version):
current_size = fake.LUN_SIZE
@ -1247,7 +1247,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
mock_get_ontap_version.assert_called_once_with(cached=True)
mock_get_lun_from_table.assert_called_once_with(fake.VOLUME['name'])
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
mock_get_lun_geometry.assert_called_once_with(
fake.LUN_METADATA['Path'])
else:
@ -1259,7 +1259,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
self.assertEqual(six.text_type(new_size_bytes),
self.library.lun_table[fake.VOLUME['name']].size)
@ddt.data('9.4', '9.6')
@ddt.data((9, 4, 0), (9, 6, 0))
def test__extend_attached_volume_direct(self, ontap_version):
current_size = fake.LUN_SIZE
@ -1296,7 +1296,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
mock_get_lun_from_table.assert_called_once_with(volume_copy['name'])
mock_get_ontap_version.assert_called_once_with(cached=True)
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
mock_get_lun_geometry.assert_called_once_with(
fake.LUN_METADATA['Path'])
else:
@ -1308,7 +1308,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
self.assertEqual(six.text_type(new_size_bytes),
self.library.lun_table[volume_copy['name']].size)
@ddt.data('9.4', '9.6')
@ddt.data((9, 4, 0), (9, 6, 0))
def test__extend_volume_clone(self, ontap_version):
current_size = fake.LUN_SIZE
@ -1342,7 +1342,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
mock_get_ontap_version.assert_called_once_with(cached=True)
mock_get_lun_from_table.assert_called_once_with(fake.VOLUME['name'])
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
self.assertFalse(mock_do_direct_resize.called)
mock_get_lun_geometry.assert_called_once_with(
fake.LUN_METADATA['Path'])
@ -1358,7 +1358,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
self.assertEqual(six.text_type(new_size_bytes),
self.library.lun_table[fake.VOLUME['name']].size)
@ddt.data('9.4', '9.6')
@ddt.data((9, 4, 0), (9, 6, 0))
def test__extend_attached_volume_clone_error(self, ontap_version):
current_size = fake.LUN_SIZE
@ -1390,7 +1390,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
self.library.lun_table = {volume_copy['name']: fake_lun}
# (throne82) This error occurs only with versions older than 9.5
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
self.assertRaises(exception.VolumeBackendAPIException,
self.library._extend_volume,
volume_copy,
@ -1416,7 +1416,7 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
mock_get_lun_from_table.assert_called_once_with(
volume_copy['name'])
@ddt.data('9.4', '9.6')
@ddt.data((9, 4, 0), (9, 6, 0))
def test__extend_volume_no_change(self, ontap_version):
current_size = fake.LUN_SIZE

2
cinder/volume/drivers/netapp/dataontap/block_base.py
View File

@ -614,7 +614,7 @@ class NetAppBlockStorageLibrary(object):
ontap_version = self.zapi_client.get_ontap_version(cached=True)
if ontap_version >= '9.5':
if ontap_version >= (9, 5, 0):
self.zapi_client.do_direct_resize(path, new_size_bytes)
else:
lun_geometry = self.zapi_client.get_lun_geometry(path)

21
cinder/volume/drivers/netapp/dataontap/client/client_base.py
View File

@ -63,25 +63,26 @@ class Client(object):
self.features = na_utils.Features()
def get_ontap_version(self, cached=True):
"""Gets the ONTAP version."""
"""Gets the ONTAP version-string and version-tuple"""
if cached:
return self.connection.get_ontap_version()
ontap_version = netapp_api.NaElement("system-get-version")
result = self.connection.invoke_successfully(ontap_version, True)
result = self.connection.invoke_successfully(
ontap_version, enable_tunneling=True)
version_tuple = result.get_child_by_name(
'version-tuple') or netapp_api.NaElement('none')
system_version_tuple = version_tuple.get_child_by_name(
ontap_version_tuple = version_tuple.get_child_by_name(
'system-version-tuple') or netapp_api.NaElement('none')
generation = system_version_tuple.get_child_content("generation")
major = system_version_tuple.get_child_content("major")
version = (
int(ontap_version_tuple.get_child_content('generation')),
int(ontap_version_tuple.get_child_content('major')),
int(ontap_version_tuple.get_child_content('minor')))
return '%(generation)s.%(major)s' % {
'generation': generation,
'major': major}
return version
def get_ontapi_version(self, cached=True):
"""Gets the supported ontapi version."""
@ -120,7 +121,7 @@ class Client(object):
# geometry on max_resize_size. In order to remove this
# limitation we create the LUN with its maximum possible size
# and then shrink to the requested size.
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
initial_size = MAX_SIZE_FOR_A_LUN
# In order to create a LUN with its maximum size (16TB),
# the space_reservation needs to be disabled
@ -153,7 +154,7 @@ class Client(object):
'volume_name': volume_name,
'ex': ex})
if ontap_version < '9.5':
if ontap_version < (9, 5, 0):
self.do_direct_resize(path, six.text_type(size))
if metadata['SpaceReserved'] == 'true':
self.set_lun_space_reservation(path, True)

7
releasenotes/notes/bug-1955057-fix-get-ontap-version-4d9fa1f6c5d2eaf3.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- |
NetApp ONTAP driver `bug #1955057
<https://bugs.launchpad.net/cinder/+bug/1955057>`_: Fixed
the function get_ontap_version on Cinder NetApp driver, now it returns a
tuple of integers instead of a string.