From a17c896c296c8f69f428077725d9f3052f30daad Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Mon, 4 Mar 2019 08:27:35 +0100 Subject: [PATCH] Add gate jobs Fix a few discrepancies discovered during unit testing. Add missing unit tests. Add end to end functional test bundle. Change-Id: Ic05c72f9e684f615b60a3975779e76526a0c9c64 --- .gitignore | 3 + .gitreview | 4 + .zuul.yaml | 3 + src/lib/charm/openstack/ceph_rbd_mirror.py | 1 - src/reactive/ceph_rbd_mirror_handlers.py | 16 +- src/tests/bundles/bionic-queens-e2e.yaml | 118 +++++++++++ src/tests/bundles/bionic-queens.yaml | 4 +- src/tests/tests.yaml | 7 +- test-requirements.txt | 2 +- tox.ini | 46 +++- unit_tests/test_ceph_rbd_mirror_handlers.py | 197 ++++++++++++++++++ ...est_lib_charm_openstack_ceph_rbd_mirror.py | 57 ++++- 12 files changed, 434 insertions(+), 24 deletions(-) create mode 100644 .gitreview create mode 100644 .zuul.yaml create mode 100644 src/tests/bundles/bionic-queens-e2e.yaml create mode 100644 unit_tests/test_ceph_rbd_mirror_handlers.py diff --git a/.gitignore b/.gitignore index eb1cd65..bcfbbf6 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,6 @@ *__pycache__* *.pyc build +.coverage +cover/ +*.swp diff --git a/.gitreview b/.gitreview new file mode 100644 index 0000000..b094119 --- /dev/null +++ b/.gitreview @@ -0,0 +1,4 @@ +[gerrit] +host=review.openstack.org +port=29418 +project=openstack/charm-ceph-rbd-mirror.git diff --git a/.zuul.yaml b/.zuul.yaml new file mode 100644 index 0000000..7051aee --- /dev/null +++ b/.zuul.yaml @@ -0,0 +1,3 @@ +- project: + templates: + - python35-charm-jobs diff --git a/src/lib/charm/openstack/ceph_rbd_mirror.py b/src/lib/charm/openstack/ceph_rbd_mirror.py index 9c50739..b765e2a 100644 --- a/src/lib/charm/openstack/ceph_rbd_mirror.py +++ b/src/lib/charm/openstack/ceph_rbd_mirror.py @@ -69,7 +69,6 @@ class CephRBDMirrorCharm(charms_openstack.plugins.CephCharm): ch_core.hookenv.log('DEBUG: mirror_pool_status({}) = "{}"' .format(pool, status), level=ch_core.hookenv.INFO) - return 'active', 'Custom' return None, None def _mirror_pool_info(self, pool): diff --git a/src/reactive/ceph_rbd_mirror_handlers.py b/src/reactive/ceph_rbd_mirror_handlers.py index 28487ad..6ba018c 100644 --- a/src/reactive/ceph_rbd_mirror_handlers.py +++ b/src/reactive/ceph_rbd_mirror_handlers.py @@ -34,12 +34,12 @@ charm.use_defaults( def request_keys(): with charm.provide_charm_instance() as charm_instance: for flag in ('ceph-local.connected', 'ceph-remote.connected'): - endpoint = reactive.relations.endpoint_from_flag(flag) + endpoint = reactive.endpoint_from_flag(flag) ch_core.hookenv.log('Ceph endpoint "{}" connected, requesting key' .format(endpoint.endpoint_name), level=ch_core.hookenv.INFO) endpoint.request_key() - charm_instance.assess_status() + charm_instance.assess_status() @reactive.when('config.changed') @@ -48,8 +48,8 @@ def request_keys(): def config_changed(): with charm.provide_charm_instance() as charm_instance: charm_instance.upgrade_if_available([ - reactive.relations.endpoint_from_flag('ceph-local.available'), - reactive.relations.endpoint_from_flag('ceph-remote.available'), + reactive.endpoint_from_flag('ceph-local.available'), + reactive.endpoint_from_flag('ceph-remote.available'), ]) charm_instance.assess_status() @@ -60,6 +60,7 @@ def disable_services(): for service in charm_instance.services: ch_core.host.service('disable', service) ch_core.host.service('stop', service) + charm_instance.assess_status() @reactive.when('ceph-local.available') @@ -78,10 +79,9 @@ def render_stuff(*args): charm_instance.configure_ceph_keyring(endpoint, cluster_name=cluster_name) charm_instance.render_with_interfaces(args) - with charm.provide_charm_instance() as charm_instance: - for service in charm_instance.services: - ch_core.host.service('enable', service) - ch_core.host.service('start', service) + for service in charm_instance.services: + ch_core.host.service('enable', service) + ch_core.host.service('start', service) reactive.set_flag('config.rendered') charm_instance.assess_status() diff --git a/src/tests/bundles/bionic-queens-e2e.yaml b/src/tests/bundles/bionic-queens-e2e.yaml new file mode 100644 index 0000000..835f4db --- /dev/null +++ b/src/tests/bundles/bionic-queens-e2e.yaml @@ -0,0 +1,118 @@ +series: bionic +applications: + mysql: + charm: cs:~openstack-charmers-next/percona-cluster + num_units: 1 + keystone: + charm: cs:~openstack-charmers-next/keystone + num_units: 1 + rabbitmq-server: + charm: cs:~openstack-charmers-next/rabbitmq-server + num_units: 1 + cinder: + charm: cs:~openstack-charmers-next/cinder + num_units: 1 + cinder-ceph: + charm: cs:~openstack-charmers-next/cinder-ceph + num_units: 0 + glance: + charm: cs:~openstack-charmers-next/glance + num_units: 1 + neutron-openvswitch: + charm: cs:~openstack-charmers-next/neutron-openvswitch + num_units: 0 + nova-cloud-controller: + charm: cs:~openstack-charmers-next/nova-cloud-controller + num_units: 1 + nova-compute: + charm: cs:~openstack-charmers-next/nova-compute + num_units: 1 + ceph-mon: + charm: cs:~openstack-charmers-next/ceph-mon + num_units: 3 + options: + expected-osd-count: 3 + source: distro + ceph-osd: + charm: cs:~openstack-charmers-next/ceph-osd + num_units: 3 + options: + source: distro + storage: + osd-devices: cinder,20G + ceph-rbd-mirror: + series: bionic + charm: ../../../ceph-rbd-mirror + num_units: 1 + options: + source: distro + ceph-mon-b: + charm: cs:~openstack-charmers-next/ceph-mon + num_units: 3 + options: + expected-osd-count: 3 + source: distro + ceph-osd-b: + charm: cs:~openstack-charmers-next/ceph-osd + num_units: 3 + options: + source: distro + storage: + osd-devices: cinder,20G + ceph-rbd-mirror-b: + series: bionic + charm: ../../../ceph-rbd-mirror + num_units: 1 + options: + source: distro +relations: +- - mysql + - keystone +- - mysql + - cinder +- - rabbitmq-server + - cinder +- - keystone + - cinder +- - cinder + - cinder-ceph +- - cinder-ceph + - ceph-mon +- - ceph-mon:osd + - ceph-osd:mon +- - ceph-mon + - ceph-rbd-mirror:ceph-local +- - ceph-mon + - ceph-rbd-mirror-b:ceph-remote +- - ceph-mon-b:osd + - ceph-osd-b:mon +- - ceph-mon-b + - ceph-rbd-mirror-b:ceph-local +- - ceph-mon-b + - ceph-rbd-mirror:ceph-remote +- - mysql:shared-db + - nova-cloud-controller:shared-db +- - keystone:identity-service + - nova-cloud-controller:identity-service +- - rabbitmq-server:amqp + - nova-cloud-controller:amqp +- - nova-compute:ceph-access + - cinder-ceph:ceph-access +- - nova-compute:amqp + - rabbitmq-server:amqp +- - nova-compute:cloud-compute + - nova-cloud-controller:cloud-compute +- - glance:identity-service + - keystone:identity-service +- - glance:shared-db + - mysql:shared-db +- - glance:amqp + - rabbitmq-server:amqp +- - glance:image-service + - nova-compute:image-service +- - neutron-openvswitch:neutron-plugin + - nova-compute:neutron-plugin +- - neutron-openvswitch:amqp + - rabbitmq-server:amqp +- - nova-cloud-controller:image-service + - glance:image-service diff --git a/src/tests/bundles/bionic-queens.yaml b/src/tests/bundles/bionic-queens.yaml index 06f41ac..f9c1224 100644 --- a/src/tests/bundles/bionic-queens.yaml +++ b/src/tests/bundles/bionic-queens.yaml @@ -16,7 +16,7 @@ applications: charm: cs:~openstack-charmers-next/cinder-ceph num_units: 0 ceph-mon: - charm: cs:~fnordahl/ceph-mon-rbd-mirror + charm: cs:~openstack-charmers-next/ceph-mon num_units: 3 options: expected-osd-count: 3 @@ -35,7 +35,7 @@ applications: options: source: distro ceph-mon-b: - charm: cs:~fnordahl/ceph-mon-rbd-mirror + charm: cs:~openstack-charmers-next/ceph-mon num_units: 3 options: expected-osd-count: 3 diff --git a/src/tests/tests.yaml b/src/tests/tests.yaml index 7cf75ff..ad4e96b 100644 --- a/src/tests/tests.yaml +++ b/src/tests/tests.yaml @@ -2,13 +2,8 @@ charm_name: ceph-rbd-mirror smoke_bundles: - bionic-queens gate_bundles: -- xenial-pike -- xenial-queens - bionic-queens -- bionic-rocky -- cosmic-rocky -dev_bundles: -- disco-stein +- bionic-queens-e2e configure: - zaza.charm_tests.noop.setup.basic_setup tests: diff --git a/test-requirements.txt b/test-requirements.txt index ca62003..3162b0f 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -4,7 +4,7 @@ # # Lint and unit test requirements flake8>=2.2.4,<=2.4.1 -os-testr>=0.4.1 +stestr requests>=2.18.4 charms.reactive mock>=1.2 diff --git a/tox.ini b/tox.ini index 41ba42d..550f5ab 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,7 @@ setenv = VIRTUAL_ENV={envdir} LAYER_PATH={toxinidir}/layers INTERFACE_PATH={toxinidir}/interfaces JUJU_REPOSITORY={toxinidir}/build -passenv = http_proxy https_proxy INTERFACE_PATH +passenv = http_proxy https_proxy install_command = pip install {opts} {packages} deps = @@ -26,17 +26,45 @@ commands = [testenv:py3] basepython = python3 deps = -r{toxinidir}/test-requirements.txt -commands = ostestr {posargs} +setenv = + {[testenv]setenv} + PYTHON=coverage run +commands = + coverage erase + stestr run {posargs} + coverage combine + coverage html -d cover + coverage xml -o cover/coverage.xml + coverage report + [testenv:py35] basepython = python3.5 deps = -r{toxinidir}/test-requirements.txt -commands = ostestr {posargs} +setenv = + {[testenv]setenv} + PYTHON=coverage run +commands = + coverage erase + stestr run {posargs} + coverage combine + coverage html -d cover + coverage xml -o cover/coverage.xml + coverage report [testenv:py36] basepython = python3.6 deps = -r{toxinidir}/test-requirements.txt -commands = ostestr {posargs} +setenv = + {[testenv]setenv} + PYTHON=coverage run +commands = + coverage erase + stestr run {posargs} + coverage combine + coverage html -d cover + coverage xml -o cover/coverage.xml + coverage report [testenv:pep8] basepython = python3 @@ -47,6 +75,16 @@ commands = flake8 {posargs} src unit_tests basepython = python3 commands = {posargs} +[coverage:run] +branch = True +concurrency = multiprocessing +parallel = True +source = + . +omit = + .tox/* + unit_tests/* + [flake8] # E402 ignore necessary for path append before sys module import in actions ignore = E402 diff --git a/unit_tests/test_ceph_rbd_mirror_handlers.py b/unit_tests/test_ceph_rbd_mirror_handlers.py new file mode 100644 index 0000000..0b5a5f7 --- /dev/null +++ b/unit_tests/test_ceph_rbd_mirror_handlers.py @@ -0,0 +1,197 @@ +# Copyright 2019 Canonical Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import mock + +import charm.openstack.ceph_rbd_mirror as crm +import reactive.ceph_rbd_mirror_handlers as handlers + +import charms_openstack.test_utils as test_utils + + +class TestRegisteredHooks(test_utils.TestRegisteredHooks): + + def test_hooks(self): + defaults = [ + 'charm.installed', + 'update-status', + 'upgrade-charm', + ] + hook_set = { + 'when': { + 'config_changed': ( + 'config.changed', + 'ceph-local.available', + 'ceph-remote.available', + ), + 'render_stuff': ( + 'ceph-local.available', + 'ceph-remote.available', + ), + 'configure_pools': ( + 'leadership.is_leader', + 'config.rendered', + 'ceph-local.available', + 'ceph-remote.available', + ), + }, + 'when_all': { + 'request_keys': ( + 'ceph-local.connected', + 'ceph-remote.connected', + ), + }, + 'when_not': { + 'disable_services': ( + 'config.rendered', + ), + }, + 'when_not_all': { + 'request_keys': ( + 'ceph-local.available', + 'ceph-remote.available', + ), + }, + } + # test that the hooks were registered + self.registered_hooks_test_helper(handlers, hook_set, defaults) + + +class TestCephRBDMirrorHandlers(test_utils.PatchHelper): + + def setUp(self): + super().setUp() + self.patch_release(crm.CephRBDMirrorCharm.release) + self.crm_charm = mock.MagicMock() + self.patch_object(handlers.charm, 'provide_charm_instance', + new=mock.MagicMock()) + self.provide_charm_instance().__enter__.return_value = \ + self.crm_charm + self.provide_charm_instance().__exit__.return_value = None + + def test_request_keys(self): + self.patch_object(handlers.reactive, 'endpoint_from_flag') + endpoint_local = mock.MagicMock() + endpoint_remote = mock.MagicMock() + endpoint_local.endpoint_name = 'ceph-local' + endpoint_remote.endpoint_name = 'ceph-remote' + self.endpoint_from_flag.side_effect = [endpoint_local, + endpoint_remote] + handlers.request_keys() + self.endpoint_from_flag.assert_has_calls([ + mock.call('ceph-local.connected'), + mock.call('ceph-remote.connected'), + ]) + endpoint_local.request_key.assert_called_once_with() + endpoint_remote.request_key.assert_called_once_with() + self.crm_charm.assess_status.assert_called_once_with() + + def test_config_changed(self): + self.patch_object(handlers.reactive, 'endpoint_from_flag') + handlers.config_changed() + self.endpoint_from_flag.assert_has_calls([ + mock.call('ceph-local.available'), + mock.call('ceph-remote.available'), + ]) + self.crm_charm.upgrade_if_available.assert_called_once_with( + [self.endpoint_from_flag(), self.endpoint_from_flag()]) + self.crm_charm.assess_status.assert_called_once_with() + + def test_disable_services(self): + self.patch_object(handlers.ch_core.host, 'service') + self.crm_charm.services = ['aservice'] + handlers.disable_services() + self.service.assert_has_calls([ + mock.call('disable', 'aservice'), + mock.call('stop', 'aservice'), + ]) + self.crm_charm.assess_status.assert_called_once_with() + + def test_render_stuff(self): + self.patch_object(handlers.ch_core.host, 'service') + endpoint_local = mock.MagicMock() + endpoint_remote = mock.MagicMock() + endpoint_local.endpoint_name = 'ceph-local' + endpoint_local.pools = {} + endpoint_remote.endpoint_name = 'ceph-remote' + endpoint_remote.pools = {} + self.crm_charm.services = ['aservice'] + handlers.render_stuff(endpoint_local, endpoint_remote) + self.crm_charm.configure_ceph_keyring.assert_has_calls([ + mock.call(endpoint_local, cluster_name=None), + mock.call(endpoint_remote, cluster_name='remote'), + ]) + self.crm_charm.render_with_interfaces.assert_called_once_with( + (endpoint_local, endpoint_remote)) + self.service.assert_has_calls([ + mock.call('enable', 'aservice'), + mock.call('start', 'aservice'), + ]) + self.crm_charm.assess_status.assert_called_once_with() + + def test_configure_pools(self): + self.patch_object(handlers.reactive, 'endpoint_from_flag') + endpoint_local = mock.MagicMock() + endpoint_remote = mock.MagicMock() + endpoint_local.endpoint_name = 'ceph-local' + endpoint_local.pools = { + 'cinder-ceph': { + 'applications': {'rbd': {}}, + 'parameters': {'pg_num': 42, 'size': 3}, + 'quota': {'max_bytes': 1024, 'max_objects': 51}, + }, + } + endpoint_remote.endpoint_name = 'ceph-remote' + self.endpoint_from_flag.side_effect = [endpoint_local, + endpoint_remote] + self.crm_charm.mirror_pool_enabled.return_value = False + handlers.configure_pools() + self.endpoint_from_flag.assert_has_calls([ + mock.call('ceph-local.available'), + mock.call('ceph-remote.available'), + ]) + self.crm_charm.mirror_pool_enabled.assert_called_once_with( + 'cinder-ceph') + self.crm_charm.mirror_pool_enable.assert_called_once_with( + 'cinder-ceph') + endpoint_remote.create_replicated_pool.assert_called_once_with( + 'cinder-ceph', replicas=3, pg_num=42, app_name='rbd', + max_bytes=1024, max_objects=51) + self.assertFalse(endpoint_remote.create_erasure_pool.called) + self.endpoint_from_flag.side_effect = [endpoint_local, + endpoint_remote] + self.crm_charm.mirror_pool_enabled.return_value = True + self.crm_charm.mirror_pool_has_peers.return_value = True + self.crm_charm.mirror_pool_enabled.reset_mock() + self.crm_charm.mirror_pool_enable.reset_mock() + handlers.configure_pools() + self.crm_charm.mirror_pool_enabled.assert_called_once_with( + 'cinder-ceph') + self.crm_charm.mirror_pool_has_peers.assert_called_once_with( + 'cinder-ceph') + self.assertFalse(self.crm_charm.mirror_pool_enable.called) + endpoint_local.pools = { + 'cinder-ceph': { + 'applications': {'rbd': {}}, + 'parameters': {'pg_num': 42, 'erasure_code_profile': 'prof'}, + 'quota': {'max_bytes': 1024, 'max_objects': 51}, + }, + } + self.endpoint_from_flag.side_effect = [endpoint_local, + endpoint_remote] + endpoint_remote.create_replicated_pool.reset_mock() + handlers.configure_pools() + endpoint_remote.create_erasure_pool.assert_called_once_with( + 'cinder-ceph', erasure_profile='prof', pg_num=42, app_name='rbd', + max_bytes=1024, max_objects=51) diff --git a/unit_tests/test_lib_charm_openstack_ceph_rbd_mirror.py b/unit_tests/test_lib_charm_openstack_ceph_rbd_mirror.py index 7e2010c..3d399f3 100644 --- a/unit_tests/test_lib_charm_openstack_ceph_rbd_mirror.py +++ b/unit_tests/test_lib_charm_openstack_ceph_rbd_mirror.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import mock + import charms_openstack.test_utils as test_utils import charm.openstack.ceph_rbd_mirror as ceph_rbd_mirror @@ -25,5 +27,56 @@ class Helper(test_utils.PatchHelper): class TestCephRBDMirrorCharm(Helper): - def test_foo(self): - pass + + def test_custom_assess_status_check(self): + self.patch_object(ceph_rbd_mirror.socket, 'gethostname') + self.patch_object(ceph_rbd_mirror.reactive, 'is_flag_set') + self.is_flag_set.return_value = False + crmc = ceph_rbd_mirror.CephRBDMirrorCharm() + self.assertEqual(crmc.custom_assess_status_check(), (None, None)) + self.is_flag_set.return_value = True + self.patch_object(ceph_rbd_mirror.reactive, 'endpoint_from_flag') + self.assertEqual(crmc.custom_assess_status_check(), + (None, None)) + self.endpoint_from_flag.assert_called_once_with( + 'ceph-local.available') + + def test__mirror_pool_info(self): + self.patch_object(ceph_rbd_mirror.socket, 'gethostname') + self.patch_object(ceph_rbd_mirror.subprocess, 'check_output') + self.gethostname.return_value = 'ahostname' + crmc = ceph_rbd_mirror.CephRBDMirrorCharm() + crmc._mirror_pool_info('apool') + self.check_output.assert_called_once_with( + ['rbd', '--id', 'rbd-mirror.ahostname', 'mirror', 'pool', 'info', + 'apool'], universal_newlines=True) + + def test_mirror_pool_enabled(self): + self.patch_object(ceph_rbd_mirror.socket, 'gethostname') + crmc = ceph_rbd_mirror.CephRBDMirrorCharm() + _mirror_pool_info = mock.MagicMock() + _mirror_pool_info.return_value = ( + 'Mode: pool\n' + 'Peers: \n' + ' UUID NAME CLIENT' + ' \n') + crmc._mirror_pool_info = _mirror_pool_info + self.assertTrue(crmc.mirror_pool_enabled('apool')) + _mirror_pool_info.assert_called_once_with('apool') + _mirror_pool_info.return_value = 'Mode: disabled\n' + self.assertFalse(crmc.mirror_pool_enabled('apool')) + + def test_mirror_pool_has_peers(self): + self.patch_object(ceph_rbd_mirror.socket, 'gethostname') + crmc = ceph_rbd_mirror.CephRBDMirrorCharm() + _mirror_pool_info = mock.MagicMock() + _mirror_pool_info.return_value = ( + 'Mode: pool\n' + 'Peers: \n' + ' UUID NAME CLIENT' + ' \n') + crmc._mirror_pool_info = _mirror_pool_info + self.assertTrue(crmc.mirror_pool_has_peers('apool')) + _mirror_pool_info.assert_called_once_with('apool') + _mirror_pool_info.return_value = 'Mode: pool\nPeers: none\n' + self.assertFalse(crmc.mirror_pool_has_peers('apool'))