openstack
/
charm-designate-bind
Code Issues Proposed changes

Fix charm so that on focal it uses named service 

This is a cherry-pick/squash of three commits:

 8949ab2 Switch Helper to use c.openstack PatchHelper
 ba7e3ac Fix charm so that on focal it uses named service
 5205bce Fix charm so that on focal it uses named service - 2

For the purposes of the backport, they have been squashed into this
single commit.  The Change-Id is from ba7e3ac.

Original commit messages (minus change ids):

ba7e3ac Fix charm so that on focal it uses named service

Pre-focal the service was called 'bind9'.  On focal+ it is called
'named'.  This patch introduces a number of changes to make this
possible:

 * Modify existing charm class "DesignateBindCharm" to include a check
   to see if the distro version is "focal" or higher; if so switch
   the service name to 'named'.

8949ab2 Switch Helper to use c.openstack PatchHelper

Switch Helper to use c.openstack PatchHelper

This charms.openstack PatchHelper class is a better version of the
Helper class in test_lib_charm_openstack_designate_bind.py as it allows
patching by fully qualified class string.  This simplifies the patching
in the tests.

Change-Id: Ida65c620fa81715d37e468217aea28f160cf31ee
Closes-Bug: #1929770
This commit is contained in:
Alex Kavanagh 2021-06-02 10:13:42 +01:00
parent de97dc60bd
commit cc2629b514
2 changed files with 138 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/lib/charm/openstack/designate_bind.py
View File

@ -236,8 +236,28 @@ class DesignateBindCharm(openstack_charm.OpenStackCharm):
group = 'bind'
def __init__(self, release=None, **kwargs):
self._ensure_override_service_name()
super(DesignateBindCharm, self).__init__(release='icehouse', **kwargs)
@classmethod
def _ensure_override_service_name(cls):
"""Override the bind9 service to named for focal+
This class is instantiated for all versions of Ubuntu, but we also need
to check for the host being focal to override the named service to
'named'.
"""
release = host.CompareHostReleases(
host.lsb_release()['DISTRIB_CODENAME'])
if release >= 'focal':
cls.services = ['named']
cls.restart_map = {
'/etc/bind/named.conf.options': cls.services,
'/etc/bind/named.conf': cls.services,
'/etc/bind/rndc.key': cls.services,
}
cls.default_service = 'named'
@staticmethod
def get_rndc_algorithm():
"""Algorithm used to encode rndc secret
@ -386,10 +406,10 @@ class DesignateBindCharm(openstack_charm.OpenStackCharm):
sync_dir = self.setup_sync_dir(sync_time)
self.create_sync_src_info_file()
# FIXME Try freezing DNS rather than stopping bind
self.service_control('stop', ['bind9'])
self.service_control('stop', [self.default_service])
tar_file = '{}/{}.tar.gz'.format(sync_dir, sync_time)
self.create_zone_tarball(tar_file)
self.service_control('start', ['bind9'])
self.service_control('start', [self.default_service])
self.set_sync_info(sync_time, '{}.tar.gz'.format(sync_time))
def service_control(self, cmd, services):
@ -461,12 +481,12 @@ class DesignateBindCharm(openstack_charm.OpenStackCharm):
else:
url = DesignateBindCharm.get_sync_src()
if url:
self.service_control('stop', ['bind9'])
self.service_control('stop', [self.default_service])
self.wget_file(url, ZONE_DIR)
tar_file = url.split('/')[-1]
subprocess.check_call(['tar', 'xf', tar_file], cwd=ZONE_DIR)
os.remove('{}/{}'.format(ZONE_DIR, tar_file))
self.service_control('start', ['bind9'])
self.service_control('start', [self.default_service])
reactive.remove_state('sync.request.sent')
reactive.set_state('zones.initialised')
else:

216
unit_tests/test_lib_charm_openstack_designate_bind.py
View File

@ -13,8 +13,8 @@
# limitations under the License.
import mock
import unittest
import charms_openstack.test_utils as test_utils
import charm.openstack.designate_bind as designate_bind
@ -27,107 +27,113 @@ def FakeConfig(init_dict):
return _config
class Helper(unittest.TestCase):
class Helper(test_utils.PatchHelper):
def setUp(self):
self._patches = {}
self._patches_start = {}
self.ch_config_patch = mock.patch('charmhelpers.core.hookenv.config')
self.ch_config = self.ch_config_patch.start()
super().setUp()
self.patch('charmhelpers.core.hookenv.config', name='ch_config')
self.ch_config.side_effect = lambda: {'ssl_param': None}
def tearDown(self):
for k, v in self._patches.items():
v.stop()
setattr(self, k, None)
self._patches = None
self._patches_start = None
self.ch_config_patch.stop()
def patch(self, obj, attr, return_value=None, **kwargs):
mocked = mock.patch.object(obj, attr, **kwargs)
self._patches[attr] = mocked
started = mocked.start()
started.return_value = return_value
self._patches_start[attr] = started
setattr(self, attr, started)
def patch_object(self, obj, attr, return_value=None, name=None, new=None):
if name is None:
name = attr
if new is not None:
mocked = mock.patch.object(obj, attr, new=new)
else:
mocked = mock.patch.object(obj, attr)
self._patches[name] = mocked
started = mocked.start()
if new is None:
started.return_value = return_value
self._patches_start[name] = started
setattr(self, name, started)
self.patch_object(designate_bind, "host", name="ch_core_host")
self.ch_core_host.lsb_release.return_value = {
"DISTRIB_CODENAME": "bionic"
}
# simply the CompareHostReleases to just return then string for
# comparisons. (won't work for xenial/bionic)
self.ch_core_host.CompareHostReleases.side_effect = lambda x: x
self.patch('charms_openstack.charm.core._singleton', new=None)
class TestOpenStackDesignateBind(Helper):
def test_install(self):
self.patch(designate_bind.DesignateBindCharm.singleton, 'install')
charm = designate_bind.DesignateBindCharm.singleton
self.patch_object(charm, 'install')
designate_bind.install()
self.install.assert_called_once_with()
def test_service_is_bind9_pre_focal(self):
charm = designate_bind.DesignateBindCharm.singleton
self.assertEqual(charm.services, ["bind9"])
for v in charm.restart_map.values():
self.assertEqual(v, ["bind9"])
self.assertEqual(charm.default_service, "bind9")
def test_service_is_bind9_when_bionic(self):
self.ch_core_host.lsb_release.return_value = {
"DISTRIB_CODENAME": "bionic"
}
charm = designate_bind.DesignateBindCharm.singleton
self.assertEqual(charm.services, ["bind9"])
for v in charm.restart_map.values():
self.assertEqual(v, ["bind9"])
self.assertEqual(charm.default_service, "bind9")
def test_service_is_named_focal_plus(self):
self.ch_core_host.lsb_release.return_value = {
"DISTRIB_CODENAME": "focal"
}
charm = designate_bind.DesignateBindCharm.singleton
self.assertEqual(charm.services, ["named"])
for v in charm.restart_map.values():
self.assertEqual(v, ["named"])
self.assertEqual(charm.default_service, "named")
def test_init_rndckey(self):
self.patch(designate_bind.DesignateBindCharm.singleton, 'init_rndckey')
self.patch_object(
designate_bind.DesignateBindCharm.singleton, 'init_rndckey')
designate_bind.init_rndckey()
self.init_rndckey.assert_called_once_with()
def test_get_rndc_secret(self):
self.patch(
self.patch_object(
designate_bind.DesignateBindCharm.singleton,
'get_rndc_secret')
designate_bind.get_rndc_secret()
self.get_rndc_secret.assert_called_once_with()
def test_get_rndc_algorithm(self):
self.patch(
self.patch_object(
designate_bind.DesignateBindCharm.singleton,
'get_rndc_algorithm')
designate_bind.get_rndc_algorithm()
self.get_rndc_algorithm.assert_called_once_with()
def test_get_sync_time(self):
self.patch(
self.patch_object(
designate_bind.DesignateBindCharm.singleton,
'get_sync_time')
designate_bind.get_sync_time()
self.get_sync_time.assert_called_once_with()
def test_setup_sync(self):
self.patch(designate_bind.DesignateBindCharm.singleton, 'setup_sync')
self.patch_object(
designate_bind.DesignateBindCharm.singleton, 'setup_sync')
designate_bind.setup_sync()
self.setup_sync.assert_called_once_with()
def test_retrieve_zones(self):
self.patch(
self.patch_object(
designate_bind.DesignateBindCharm.singleton,
'retrieve_zones')
designate_bind.retrieve_zones('hacluster')
self.retrieve_zones.assert_called_once_with('hacluster')
def test_request_sync(self):
self.patch(
self.patch_object(
designate_bind.DesignateBindCharm.singleton,
'request_sync')
designate_bind.request_sync('hacluster')
self.request_sync.assert_called_once_with('hacluster')
def test_process_requests(self):
self.patch(
self.patch_object(
designate_bind.DesignateBindCharm.singleton,
'process_requests')
designate_bind.process_requests('hacluster')
self.process_requests.assert_called_once_with('hacluster')
def test_render_all_configs(self):
self.patch(
self.patch_object(
designate_bind.DesignateBindCharm.singleton,
'render_with_interfaces')
designate_bind.render_all_configs('interface_list')
@ -153,8 +159,8 @@ class TestDNSAdapter(Helper):
def test_control_listen_ip(self):
relation = mock.MagicMock()
self.patch(designate_bind.ch_ip, 'get_relation_ip')
self.patch(designate_bind.hookenv, 'unit_private_ip')
self.patch_object(designate_bind.ch_ip, 'get_relation_ip')
self.patch_object(designate_bind.hookenv, 'unit_private_ip')
self.get_relation_ip.return_value = 'ip1'
a = designate_bind.DNSAdapter(relation)
self.assertEqual(a.control_listen_ip, 'ip1')
@ -167,14 +173,15 @@ class TestDNSAdapter(Helper):
def test_algorithm(self):
relation = mock.MagicMock()
self.patch(designate_bind.DesignateBindCharm, 'get_rndc_algorithm')
self.patch_object(
designate_bind.DesignateBindCharm, 'get_rndc_algorithm')
self.get_rndc_algorithm.return_value = 'algo1'
a = designate_bind.DNSAdapter(relation)
self.assertEqual(a.algorithm, 'algo1')
def test_secret(self):
relation = mock.MagicMock()
self.patch(designate_bind.DesignateBindCharm, 'get_rndc_secret')
self.patch_object(designate_bind.DesignateBindCharm, 'get_rndc_secret')
self.get_rndc_secret.return_value = 'secret1'
a = designate_bind.DNSAdapter(relation)
self.assertEqual(a.secret, 'secret1')
@ -202,7 +209,7 @@ class TestDesignateBindCharm(Helper):
)
def test_get_rndc_secret(self):
self.patch(designate_bind.hookenv, 'leader_get')
self.patch_object(designate_bind.hookenv, 'leader_get')
self.leader_get.return_value = 'secret1'
self.assertEqual(
designate_bind.DesignateBindCharm.get_rndc_secret(),
@ -210,7 +217,7 @@ class TestDesignateBindCharm(Helper):
)
def test_get_sync_src(self):
self.patch(designate_bind.hookenv, 'leader_get')
self.patch_object(designate_bind.hookenv, 'leader_get')
self.leader_get.return_value = 'http://ip1/my.tar'
self.assertEqual(
designate_bind.DesignateBindCharm.get_sync_src(),
@ -218,7 +225,7 @@ class TestDesignateBindCharm(Helper):
)
def test_get_sync_time(self):
self.patch(designate_bind.hookenv, 'leader_get')
self.patch_object(designate_bind.hookenv, 'leader_get')
self.leader_get.return_value = '100'
self.assertEqual(
designate_bind.DesignateBindCharm.get_sync_time(),
@ -227,9 +234,9 @@ class TestDesignateBindCharm(Helper):
def test_process_requests(self):
hacluster = mock.MagicMock()
self.patch(designate_bind.hookenv, 'log')
self.patch(designate_bind.DesignateBindCharm, 'setup_sync')
self.patch(designate_bind.DesignateBindCharm, 'get_sync_time')
self.patch_object(designate_bind.hookenv, 'log')
self.patch_object(designate_bind.DesignateBindCharm, 'setup_sync')
self.patch_object(designate_bind.DesignateBindCharm, 'get_sync_time')
a = designate_bind.DesignateBindCharm()
# No queued requests
hacluster.retrieve_remote.return_value = []
@ -250,8 +257,8 @@ class TestDesignateBindCharm(Helper):
self.assertTrue(self.setup_sync.called)
def test_set_sync_info(self):
self.patch(designate_bind.hookenv, 'leader_set')
self.patch(designate_bind.hookenv, 'unit_private_ip')
self.patch_object(designate_bind.hookenv, 'leader_set')
self.patch_object(designate_bind.hookenv, 'unit_private_ip')
self.unit_private_ip.return_value = 'ip1'
a = designate_bind.DesignateBindCharm()
a.set_sync_info('20', '/tmp/tarball.tar')
@ -261,9 +268,10 @@ class TestDesignateBindCharm(Helper):
def test_generate_rndc_key(self):
hmac_mock = mock.MagicMock()
self.patch(designate_bind.os, 'urandom', return_value='seed')
self.patch(designate_bind.hmac, 'new', return_value=hmac_mock)
self.patch(designate_bind.base64, 'b64encode', return_value=hmac_mock)
self.patch_object(designate_bind.os, 'urandom', return_value='seed')
self.patch_object(designate_bind.hmac, 'new', return_value=hmac_mock)
self.patch_object(
designate_bind.base64, 'b64encode', return_value=hmac_mock)
self.patch_object(designate_bind.hashlib, 'md5', new='md5lib')
a = designate_bind.DesignateBindCharm()
a.generate_rndc_key()
@ -273,11 +281,12 @@ class TestDesignateBindCharm(Helper):
msg=b'RNDC Secret')
def test_init_rndckey(self):
self.patch(designate_bind.hookenv, 'log')
self.patch(designate_bind.DesignateBindCharm, 'get_rndc_secret')
self.patch(designate_bind.DesignateBindCharm, 'generate_rndc_key')
self.patch(designate_bind.hookenv, 'leader_set')
self.patch(designate_bind.hookenv, 'is_leader')
self.patch_object(designate_bind.hookenv, 'log')
self.patch_object(designate_bind.DesignateBindCharm, 'get_rndc_secret')
self.patch_object(
designate_bind.DesignateBindCharm, 'generate_rndc_key')
self.patch_object(designate_bind.hookenv, 'leader_set')
self.patch_object(designate_bind.hookenv, 'is_leader')
a = designate_bind.DesignateBindCharm()
# Test secret already stored
self.get_rndc_secret.return_value = 'mysecret'
@ -294,8 +303,8 @@ class TestDesignateBindCharm(Helper):
self.assertEqual(a.init_rndckey(), None)
def test_create_zone_tarball(self):
self.patch(designate_bind.glob, 'glob')
self.patch(designate_bind.subprocess, 'check_call')
self.patch_object(designate_bind.glob, 'glob')
self.patch_object(designate_bind.subprocess, 'check_call')
_files = {
'/var/cache/bind/juju*': ['jujufile1'],
'/var/cache/bind/slave*': ['slavefile1'],
@ -309,8 +318,8 @@ class TestDesignateBindCharm(Helper):
'nsffile', 'nsdfile'], cwd='/var/cache/bind')
def test_setup_sync_dir(self):
self.patch(designate_bind.os, 'mkdir')
self.patch(designate_bind.os, 'chmod')
self.patch_object(designate_bind.os, 'mkdir')
self.patch_object(designate_bind.os, 'chmod')
a = designate_bind.DesignateBindCharm()
a.setup_sync_dir('100')
self.mkdir.assert_called_once_with('/var/www/html/zone-syncs', 493)
@ -321,7 +330,8 @@ class TestDesignateBindCharm(Helper):
self.chmod.assert_called_once_with('/var/www/html/zone-syncs', 493)
def test_create_sync_src_info_file(self):
self.patch(designate_bind.hookenv, 'local_unit', return_value='unit/1')
self.patch_object(
designate_bind.hookenv, 'local_unit', return_value='unit/1')
a = designate_bind.DesignateBindCharm()
with mock.patch('builtins.open') as bob:
a.create_sync_src_info_file()
@ -330,15 +340,17 @@ class TestDesignateBindCharm(Helper):
'w+')
def test_setup_sync(self):
self.patch(designate_bind.hookenv, 'log')
self.patch(designate_bind.DesignateBindCharm, 'setup_sync_dir')
self.patch(designate_bind.time, 'time')
self.patch(
self.patch_object(designate_bind.hookenv, 'log')
self.patch_object(designate_bind.DesignateBindCharm, 'setup_sync_dir')
self.patch_object(designate_bind.time, 'time')
self.patch_object(
designate_bind.DesignateBindCharm,
'create_sync_src_info_file')
self.patch(designate_bind.DesignateBindCharm, 'service_control')
self.patch(designate_bind.DesignateBindCharm, 'create_zone_tarball')
self.patch(designate_bind.DesignateBindCharm, 'set_sync_info')
self.patch_object(designate_bind.DesignateBindCharm, 'service_control')
self.patch_object(
designate_bind.DesignateBindCharm, 'create_zone_tarball')
self.patch_object(
designate_bind.DesignateBindCharm, 'set_sync_info')
self.setup_sync_dir.return_value = '/tmp/zonefiles'
self.time.return_value = 100
a = designate_bind.DesignateBindCharm()
@ -354,7 +366,7 @@ class TestDesignateBindCharm(Helper):
self.set_sync_info.assert_called_once_with('100', '100.tar.gz')
def test_service_control(self):
self.patch(designate_bind.host, 'service_stop')
self.patch_object(designate_bind.host, 'service_stop')
a = designate_bind.DesignateBindCharm()
a.service_control('stop', ['svc1', 'svc2'])
ctrl_calls = [
@ -363,9 +375,9 @@ class TestDesignateBindCharm(Helper):
self.service_stop.assert_has_calls(ctrl_calls)
def test_request_sync(self):
self.patch(designate_bind.time, 'time')
self.patch_object(designate_bind.time, 'time')
relation = mock.MagicMock()
self.patch(designate_bind.reactive, 'set_state')
self.patch_object(designate_bind.reactive, 'set_state')
self.time.return_value = 100
a = designate_bind.DesignateBindCharm()
a.request_sync(relation)
@ -376,7 +388,7 @@ class TestDesignateBindCharm(Helper):
def test_wget_file(self):
# retry_on_exception patched out in __init__.py
self.patch(designate_bind.subprocess, 'check_call')
self.patch_object(designate_bind.subprocess, 'check_call')
a = designate_bind.DesignateBindCharm()
a.wget_file('http://ip1/tarfile.tar', '/tmp')
self.check_call.assert_called_once_with(
@ -387,14 +399,14 @@ class TestDesignateBindCharm(Helper):
def test_retrieve_zones_cluster_relation(self):
relation = mock.MagicMock()
self.patch(designate_bind.DesignateBindCharm, 'get_sync_time')
self.patch(designate_bind.DesignateBindCharm, 'get_sync_src')
self.patch(designate_bind.DesignateBindCharm, 'service_control')
self.patch(designate_bind.hookenv, 'log')
self.patch(designate_bind.reactive, 'set_state')
self.patch(designate_bind.reactive, 'remove_state')
self.patch(designate_bind.os, 'remove')
self.patch(designate_bind.subprocess, 'check_call')
self.patch_object(designate_bind.DesignateBindCharm, 'get_sync_time')
self.patch_object(designate_bind.DesignateBindCharm, 'get_sync_src')
self.patch_object(designate_bind.DesignateBindCharm, 'service_control')
self.patch_object(designate_bind.hookenv, 'log')
self.patch_object(designate_bind.reactive, 'set_state')
self.patch_object(designate_bind.reactive, 'remove_state')
self.patch_object(designate_bind.os, 'remove')
self.patch_object(designate_bind.subprocess, 'check_call')
self.patch_object(designate_bind.DesignateBindCharm, 'wget_file')
self.get_sync_src.return_value = 'http://ip1/tarfile.tar'
ctrl_calls = [
@ -420,10 +432,10 @@ class TestDesignateBindCharm(Helper):
def test_retrieve_zones_cluster_relation_nourl(self):
relation = mock.MagicMock()
self.patch(designate_bind.DesignateBindCharm, 'get_sync_time')
self.patch(designate_bind.DesignateBindCharm, 'get_sync_src')
self.patch_object(designate_bind.DesignateBindCharm, 'get_sync_time')
self.patch_object(designate_bind.DesignateBindCharm, 'get_sync_src')
self.patch_object(designate_bind.DesignateBindCharm, 'wget_file')
self.patch(designate_bind.hookenv, 'log')
self.patch_object(designate_bind.hookenv, 'log')
self.get_sync_src.return_value = None
relation.retrieve_local.return_value = ['10']
self.get_sync_time.return_value = '20'
@ -432,14 +444,14 @@ class TestDesignateBindCharm(Helper):
self.assertFalse(self.wget_file.called)
def test_retrieve_zones_no_cluster_relation(self):
self.patch(designate_bind.DesignateBindCharm, 'get_sync_time')
self.patch(designate_bind.DesignateBindCharm, 'get_sync_src')
self.patch(designate_bind.DesignateBindCharm, 'service_control')
self.patch(designate_bind.hookenv, 'log')
self.patch(designate_bind.reactive, 'set_state')
self.patch(designate_bind.reactive, 'remove_state')
self.patch(designate_bind.os, 'remove')
self.patch(designate_bind.subprocess, 'check_call')
self.patch_object(designate_bind.DesignateBindCharm, 'get_sync_time')
self.patch_object(designate_bind.DesignateBindCharm, 'get_sync_src')
self.patch_object(designate_bind.DesignateBindCharm, 'service_control')
self.patch_object(designate_bind.hookenv, 'log')
self.patch_object(designate_bind.reactive, 'set_state')
self.patch_object(designate_bind.reactive, 'remove_state')
self.patch_object(designate_bind.os, 'remove')
self.patch_object(designate_bind.subprocess, 'check_call')
self.patch_object(designate_bind.DesignateBindCharm, 'wget_file')
self.get_sync_src.return_value = 'http://ip1/tarfile.tar'
ctrl_calls = [
@ -456,7 +468,7 @@ class TestDesignateBindCharm(Helper):
'/var/cache/bind')
def test_set_apparmor(self):
self.patch(designate_bind.os.path, 'isfile')
self.patch_object(designate_bind.os.path, 'isfile')
a = designate_bind.DesignateBindCharm()
self.isfile.return_value = True
with mock.patch('builtins.open') as bob: