openstack
/
charm-swift-storage
Code Issues Proposed changes

WIP - Improve mocking of platform depending calls during unit test 

Patch more platform dependent functions during unit testing. Also use
the 'CHARMHELPERS_IN_UNITTEST' env var to disable platform detection in
charmhelpers.osplatform.get_platform() which is called at module load
time via decorators in the hooks/switf_storage_hooks.py file. This is
very tricky to mock out as they are called during test discovery before
mocks really get a chance to run.

Note this depends on https://github.com/juju/charm-helpers/pull/862

Change-Id: Ic5a63af9dcbcfbc5947a5e19b8ea35345bc49943
This commit is contained in:
Alex Kavanagh 2023-10-23 12:44:51 +01:00
parent 8045849848
commit e239794f86
6 changed files with 51 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
charmhelpers/osplatform.py
View File

@ -2,7 +2,7 @@ import platform
import os
def get_platform():
def _get_platform():
"""Return the current OS platform.
For example: if current os platform is Ubuntu then a string "ubuntu"
@ -47,3 +47,10 @@ def _get_platform_from_fs():
for k, v in content.items():
content[k] = v.strip('"')
return content["NAME"]
## If the unit-test mode is set, the platform is always "ubuntu"
if not os.environ.get('CHARMHELPERS_IN_UNITTEST', False):
get_platform = _get_platform
else:
get_platform = lambda: "ubuntu"

4
hooks/swift_storage_hooks.py
View File

@ -221,7 +221,7 @@ def install():
@hooks.hook('config-changed')
@pause_aware_restart_on_change(restart_map())
@pause_aware_restart_on_change(restart_map)
@harden()
def config_changed():
if config('enable-firewall'):
@ -332,7 +332,7 @@ def swift_storage_relation_joined(rid=None):
@hooks.hook('swift-storage-relation-changed')
@pause_aware_restart_on_change(restart_map())
@pause_aware_restart_on_change(restart_map)
def swift_storage_relation_changed():
setup_ufw()
rings_url = relation_get('rings_url')

10
lib/misc_utils.py
View File

@ -112,10 +112,16 @@ def is_paused():
def pause_aware_restart_on_change(restart_map):
"""Avoids restarting services if config changes when unit is paused."""
"""Avoids restarting services if config changes when unit is paused.
:param restart_map: the restart map.
:type restart_map: Dict[str, List[str,]] | Callable[[], Dict[str, Any]]
"""
def wrapper(f):
if is_paused():
return f
else:
return restart_on_change(restart_map)(f)
_restart_map = (
restart_map() if callable(restart_map) else restart_map)
return restart_on_change(_restart_map)(f)
return wrapper

1
tox.ini
View File

@ -22,6 +22,7 @@ skip_install = True
setenv = VIRTUAL_ENV={envdir}
PYTHONHASHSEED=0
CHARM_DIR={envdir}
CHARMHELPERS_IN_UNITTEST=true
commands = stestr run --slowest {posargs}
allowlist_externals =
charmcraft

14
unit_tests/test_swift_storage_context.py
View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from unittest.mock import MagicMock
from unittest.mock import patch, MagicMock
from unit_tests.test_utils import CharmTestCase, patch_open
@ -81,9 +81,13 @@ class SwiftStorageContextTests(CharmTestCase):
ctxt.enable_rsyncd()
_file.write.assert_called_with('RSYNC_ENABLE=true\n')
def test_swift_storage_lockup_timeout_too_low(self):
@patch.object(swift_context, 'lsb_release')
def test_swift_storage_lockup_timeout_too_low(self, mock_lsb_release):
self.test_config.set('object-rsync-timeout', 1000)
self.test_config.set('object-lockup-timeout', 1050)
mock_lsb_release.return_value = {
'DISTRIB_CODENAME': 'jammy'
}
ctxt = swift_context.SwiftStorageServerContext()
result = ctxt()
ex = {
@ -92,7 +96,8 @@ class SwiftStorageContextTests(CharmTestCase):
}
self.assertDictContainsSubset(ex, result)
def test_swift_storage_server_context(self):
@patch.object(swift_context, 'lsb_release')
def test_swift_storage_server_context(self, mock_lsb_release):
self.unit_private_ip.return_value = '10.0.0.5'
self.test_config.set('account-server-port', '500')
self.test_config.set('account-server-port-rep', '510')
@ -109,6 +114,9 @@ class SwiftStorageContextTests(CharmTestCase):
self.test_config.set('object-lockup-timeout', 3000)
self.test_config.set('object-handoffs-first', True)
self.test_config.set('file-allocation-reserve', '10737418240')
mock_lsb_release.return_value = {
'DISTRIB_CODENAME': 'jammy'
}
ctxt = swift_context.SwiftStorageServerContext()
result = ctxt()
ex = {

27
unit_tests/test_swift_storage_relations.py
View File

@ -102,10 +102,14 @@ class SwiftStorageRelationsTests(CharmTestCase):
hooks.config_changed()
self.add_to_updatedb_prunepath.assert_called_with("/srv/node")
@patch.object(hooks, 'get_os_codename_install_source')
@patch.object(hooks, 'add_ufw_gre_rule', lambda *args: None)
@patch('lib.swift_storage_utils.swift_init')
def test_install_hook(self, mock_swift_init):
def test_install_hook(self,
mock_swift_init,
mock_get_os_codename_install_source):
mock_swift_init.return_value = 0
mock_get_os_codename_install_source.return_value = 'havana'
self.test_config.set('openstack-origin', 'cloud:precise-havana')
hooks.install()
self.configure_installation_source.assert_called_with(
@ -186,6 +190,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
self.assertTrue(self.update_nrpe_config.called)
self.assertTrue(mock_ensure_devs_tracked.called)
@patch.object(hooks, 'enable_replication')
@patch('lib.swift_storage_utils.get_device_blkid',
lambda dev: str(uuid.uuid4()))
@patch.object(hooks.os, 'environ')
@ -197,11 +202,13 @@ class SwiftStorageRelationsTests(CharmTestCase):
@patch.object(uuid, 'uuid4', lambda: 'a-test-uuid')
def _test_storage_joined_single_device(self, mock_kvstore, mock_local_unit,
mock_rel_set, mock_environ,
mock_enable_replication,
env_key):
test_uuid = uuid.uuid4()
test_environ = {env_key: test_uuid}
mock_environ.get.side_effect = test_environ.get
mock_local_unit.return_value = 'test/0'
mock_enable_replication.return_value = True
kvstore = mock_kvstore.return_value
kvstore.__enter__.return_value = kvstore
kvstore.get.return_value = None
@ -259,6 +266,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
'''Ensure use of JUJU_MODEL_UUID for Juju >= 2'''
self._test_storage_joined_single_device(env_key='JUJU_MODEL_UUID')
@patch.object(hooks, 'enable_replication')
@patch('lib.swift_storage_utils.get_device_blkid',
lambda dev: '%s-blkid-uuid' % os.path.basename(dev))
@patch.object(hooks.os, 'environ')
@ -268,13 +276,15 @@ class SwiftStorageRelationsTests(CharmTestCase):
@patch('lib.swift_storage_utils.KVStore')
@patch.object(uuid, 'uuid4', lambda: 'a-test-uuid')
def test_storage_joined_multi_device(self, mock_kvstore, mock_local_unit,
mock_environ):
mock_environ,
mock_enable_replication):
test_uuid = uuid.uuid4()
test_environ = {'JUJU_ENV_UUID': test_uuid}
mock_environ.get.side_effect = test_environ.get
self.test_kv.set('prepared-devices', ['/dev/vdb', '/dev/vdc',
'/dev/vdd'])
mock_local_unit.return_value = 'test/0'
mock_enable_replication.return_value = True
kvstore = mock_kvstore.return_value
kvstore.__enter__.return_value = kvstore
kvstore.get.return_value = None
@ -305,6 +315,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
)
self.get_relation_ip.assert_called_once_with('swift-storage')
@patch.object(hooks, 'enable_replication')
@patch('lib.swift_storage_utils.get_device_blkid',
lambda dev: '%s-blkid-uuid' % os.path.basename(dev))
@patch.object(hooks.os, 'environ')
@ -312,16 +323,20 @@ class SwiftStorageRelationsTests(CharmTestCase):
@patch('lib.swift_storage_utils.local_unit')
@patch('lib.swift_storage_utils.relation_ids', lambda *args: [])
@patch('lib.swift_storage_utils.KVStore')
def test_storage_joined_dev_exists_unknown_juju_env_uuid(self,
mock_kvstore,
mock_local_unit,
mock_environ):
def test_storage_joined_dev_exists_unknown_juju_env_uuid(
self,
mock_kvstore,
mock_local_unit,
mock_environ,
mock_enable_replication,
):
test_uuid = uuid.uuid4()
test_environ = {'JUJU_ENV_UUID': test_uuid}
mock_environ.get.side_effect = test_environ.get
self.test_kv.set('prepared-devices', ['/dev/vdb', '/dev/vdc',
'/dev/vdd'])
mock_local_unit.return_value = 'test/0'
mock_enable_replication.return_value = True
kvstore = mock_kvstore.return_value
kvstore.__enter__.return_value = kvstore
kvstore.get.return_value = None