diff --git a/src/lib/charm/openstack/designate_bind.py b/src/lib/charm/openstack/designate_bind.py index febc030..ce888c4 100644 --- a/src/lib/charm/openstack/designate_bind.py +++ b/src/lib/charm/openstack/designate_bind.py @@ -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: diff --git a/unit_tests/test_lib_charm_openstack_designate_bind.py b/unit_tests/test_lib_charm_openstack_designate_bind.py index 83bd727..d9d736e 100644 --- a/unit_tests/test_lib_charm_openstack_designate_bind.py +++ b/unit_tests/test_lib_charm_openstack_designate_bind.py @@ -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: